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

Use [[deprecated]] to mark deprecated classes and functions #31021

Closed
Dr15Jones opened this issue Aug 2, 2020 · 7 comments
Closed

Use [[deprecated]] to mark deprecated classes and functions #31021

Dr15Jones opened this issue Aug 2, 2020 · 7 comments

Comments

@Dr15Jones
Copy link
Contributor

Both gcc 8.3+ and clang 9+ support the attrbute [[deprecated]]. We could use this to

  • mark the legacy EDFilter, EDProducer, and EDAnalyzer legacy classes
  • mark the EventSetupRecord::get calls which do not use an ESGetToken

These would likely generate a large number of warnings at first but would at least force pull requests to not use the deprecated items.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2020

A new Issue was created by @Dr15Jones Chris Jones.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

assign core

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2020

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@Dr15Jones
Copy link
Contributor Author

In order to avoid flooding the IBs with warnings, we could wrap the use of [[deprecated]] in a macro and make sure the macro is off during the IB builds but on during the pull request tests and when people build locally.

@makortel
Copy link
Contributor

makortel commented Aug 3, 2020

Being able to ignore these warnings in IBs is (also?) my main concern. In PR tests, what about code in files not touched by the PR? If we manage to ignore those, this would effectively mean that any change in an "offending" file would require a migration away (which could be ok, but would require a policy discussion first I suppose).

@makortel
Copy link
Contributor

+1

This has been implemented.

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

@qliphy qliphy closed this as completed Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants