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

STYLE: Remove legacy content from "itkMultiThreader.h" #4693

Conversation

N-Dekker
Copy link
Contributor

"itkMultiThreader.h" was deprecated already from ITK 5.0.

"itkMultiThreader.h" was deprecated already from ITK 5.0.
@github-actions github-actions bot added the type:Style Style changes: no logic impact (indentation, comments, naming) label May 23, 2024
@dzenanz dzenanz requested a review from thewtex May 23, 2024 15:01
@dzenanz
Copy link
Member

dzenanz commented May 23, 2024

@thewtex Is the next version number 5.5, or 6.0?

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

If we begin 6.0 development soon, this entire file should be deleted.

Until then, I would prefer to leave this file as-is.

@N-Dekker
Copy link
Contributor Author

Of course, this specific code change isn't very important to me. I'm just wondering now what we should do about all the "legacy support code" in general, now that 5.4 is tagged.

Clearly it's OK now to mark things "future legacy":

As well as to move things from "future legacy" to "legacy only":

So then it seems to make sense to me to also drop old "legacy only" things. But again, I'm just wondering 🤷

@dzenanz
Copy link
Member

dzenanz commented May 23, 2024

drop old "legacy only" things

This should be done consistently (for all legacy things), once we declare the begin of 6.0 development.

@dzenanz
Copy link
Member

dzenanz commented May 23, 2024

Something akin to this: 01580b3.

@N-Dekker
Copy link
Contributor Author

Thanks @dzenanz Does that mean that all legacy things are treated the same way? Being: let them be there until ITK 6, and then drop them all? (I can imagine that some legacy things are more bothering than others. For example, I would find it a pity if legacy support of #4691 cannot yet be dropped with ITK 6)

@dzenanz
Copy link
Member

dzenanz commented May 23, 2024

With a new major version start, we generally remove all the code which is currently legacy, and convert all future legacy into current legacy. Which is what you already did for a few of your favorites 😄

@blowekamp
Copy link
Member

Are we going to have some kinds of "ITKv5_COMPATIBILITY" mode?

@dzenanz
Copy link
Member

dzenanz commented May 23, 2024

Would we need anything beyond legacy enabled?

@seanm
Copy link
Contributor

seanm commented May 24, 2024

Isn't "STYLE" inappropriate? This is a breaking change, not something merely stylistic.

@thewtex
Copy link
Member

thewtex commented May 28, 2024

The next feature release is ITK 6, and it is in progress.

Removing legacy content should be ENH instead of STYLE as @seanm mentioned.

While the process is generally to eventually drop legacy code, we do not need to automatically or always remove "legacy" and update "future legacy" to "legacy". This can be handled on a case-by-case basis. If the code is still widely used, there is not a clear, straightforward path to migration, and the code is not causing issues, it can be kept.

If deprecated code is removed, an entry should be created in an ITK 6 migration guide here: https://github.com/InsightSoftwareConsortium/ITK/tree/master/Documentation/docs/migration_guides

@thewtex
Copy link
Member

thewtex commented May 28, 2024

Clarifications and initialization started in #4696 #4695

@dzenanz
Copy link
Member

dzenanz commented Jun 18, 2024

Time to close this PR and take out the deprecation removal axe? 😄

@dzenanz
Copy link
Member

dzenanz commented Jun 21, 2024

Closing in favor of #4729.

@dzenanz dzenanz closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Style Style changes: no logic impact (indentation, comments, naming)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants