Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LegacyND2Reader and options #4049

Closed
wants to merge 2 commits into from

Conversation

dgault
Copy link
Member

@dgault dgault commented Jul 14, 2023

This the follow up to #4030

  • LegacyND2Reader has been removed
  • ND2Reader has been removed
  • NativeND2Reader has been renamed to ND2Reader and deprecation flags removed
  • Options to use the LegacyReader in Bio-Formats Plugins have been removed

@dgault dgault added this to the 7.0.0 milestone Jul 14, 2023
@melissalinkert
Copy link
Member

Diff looks fine to me; only thought is it's going to be a little difficult to look at historical changes to what used to be NativeND2Reader, as git log -p ND2Reader.java or similar won't show commits on NativeND2Reader. Would it be worth doing something like:

  • git rm ND2Reader.java
  • git mv NativeND2Reader.java ND2Reader.java

so that git log --follow -p ND2Reader.java can show the history of NativeND2Reader.java?

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seconding @melissalinkert's comment, I think it would be valuable to evaluate if ND2Reader could inherit the history of NativeND2Reader from a maintenance perspective.

Two other suggestions/comments:

  • should lib/LegacyND2Reader.dll also be removed as part of this PR?
  • while reviewing Remove reference to LegacyND2 bio-formats-documentation#319, I was reminded of nativend2.chunkmap option. Would we consider renaming the option as nd2.chunkmap? And if so, would it be feasible to keep nativend2.chunkmap as a property alias (and deprecate it for 8.0.0)?

@dgault
Copy link
Member Author

dgault commented Jul 17, 2023

Closing this to open a fresh PR that retains the NativeND2Reader history for ND2Reader

@dgault dgault closed this Jul 17, 2023
@dgault dgault removed this from the 7.0.0 milestone Jul 17, 2023
@dgault dgault mentioned this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants