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

Annotating deprecated names with a version number. #9743

Closed
bangerth opened this issue Mar 26, 2020 · 7 comments
Closed

Annotating deprecated names with a version number. #9743

bangerth opened this issue Mar 26, 2020 · 7 comments

Comments

@bangerth
Copy link
Member

#9742 reminded me of something I had thought about on a number of occasions before: When we deprecate functions or classes, we just plop a DEAL_II_DEPRECATED in front of it. But when coming back to it later and thinking about whether we can remove something, it's not always clear when the deprecation actually happened: Before or after the previous release?

It would be useful if we could have an annotation that explains this. For example, DEAL_II_DEPRECATED_VERSION(9,1) would mean that the name in question was deprecated in time for the 9.1 release, and can be removed after that release. Any thoughts?

@masterleinad
Copy link
Member

I agree that that might be useful. Admittedly, we can at least generate that information through contrib/utilities/print_deprecated.py.

@tjhei
Copy link
Member

tjhei commented Mar 26, 2020

Along these lines: the [[deprecated]] attribute can have a message that is conveniently displayed when you use it. This could be used for the version and also a short message describing the alternative.

@kronbichler
Copy link
Member

I like this idea.

For example, DEAL_II_DEPRECATED_VERSION(9,1) would mean that the name in question was deprecated in time for the 9.1 release, and can be removed after that release.

As I have complained about this before, this also gives us a chance to establish an understandable cadence with removing deprecated features. If we keep things around for 1 release cycle, my favorite to make the cut would be 1-3 months before a release. Doing the cut soon after the release has the chance that a feature can be deprecated for a few weeks only until it gets removed (in the extreme case we deprecated it close before the release), which I find uncomfortably short notice. In any case, having a clear procedure avoids disappointment or case-by-case discussions.

@masterleinad
Copy link
Member

I think that we should try to get rid of all deprecations in 9.0. After that, we can annotate the (hopefully) not so many deprecations in 9.1.

@drwells
Copy link
Member

drwells commented Mar 28, 2020

print_deprecations.py will print the release corresponding to the last commit touching a line including DEAL_II_DEPRECATED - the script isn't perfect (running clang-format moved a few statements forward a release) but I don't think its necessary to manually enter this information ourselves. For example, here are the first few relevant lines of output:

File: ./include/deal.II/base/mg_level_object.h:121: 
Line: DEAL_II_DEPRECATED
Date: 2017-11-13 22:41:00
Message: Fix position of DEAL_II_DEPRECATED
Hash: 9eb7ba3ab5140cb505367dae7b3ee7bf5b295c38
Release: 9.0.0

File: ./include/deal.II/base/timer.h:151: 
Line: DEAL_II_DEPRECATED
Date: 2017-11-13 22:41:00
Message: Fix position of DEAL_II_DEPRECATED
Hash: 9eb7ba3ab5140cb505367dae7b3ee7bf5b295c38
Release: 9.0.0

File: ./include/deal.II/base/timer.h:173: 
Line: DEAL_II_DEPRECATED
Date: 2017-11-13 22:41:00
Message: Fix position of DEAL_II_DEPRECATED
Hash: 9eb7ba3ab5140cb505367dae7b3ee7bf5b295c38
Release: 9.0.0

Hence we know that those items (@masterleinad already got the timer functions) were deprecated prior to 9.0.0.

Along these lines: the [[deprecated]] attribute can have a message that is conveniently displayed when you use it. This could be used for the version and also a short message describing the alternative.

👍

I think it makes sense to vote on a clear procedure for deprecations. What about 🚀 for 1 release and 👍 for two?

@bangerth
Copy link
Member Author

I had not known about print_deprecations.py. That makes things simpler and I'm happy to just close this PR again unless there is strong support for it. (It would be nice to include the version number in the compiler warning, though, using the string attachment to the attribute @tjhei dug up.)

If you want to have a good chuckle, look through the completely non-deterministic places in which the [[deprecated]] attribute has to be put for the various kinds of declarations in https://en.cppreference.com/w/cpp/language/attributes/deprecated ...

@drwells
Copy link
Member

drwells commented Jun 11, 2021

I think we addressed this well enough with DEAL_II_DEPRECATED_EARLY.

@drwells drwells closed this as completed Jun 11, 2021
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

No branches or pull requests

5 participants