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

Deprecate getters and setters for deprecated configuration properties consistently #20812

Closed
mbhave opened this issue Apr 2, 2020 · 7 comments
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@mbhave
Copy link
Contributor

mbhave commented Apr 2, 2020

When we deprecate a configuration property, we add @DeprecatedConfigurationProperty to the getter along with a @Deprecated on the getter and setter. We don't do this consistently though. This is one example where the getter and setter don't have @Deprecated.

@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Apr 2, 2020
@wilkinsona wilkinsona added type: documentation A documentation update and removed for: team-attention An issue we'd like other members of the team to review labels Apr 3, 2020
@wilkinsona wilkinsona added this to the 2.2.x milestone Apr 3, 2020
@shahaf-sameach
Copy link

hi, would like to be a new contributor, can I take this for starters?

@snicoll
Copy link
Member

snicoll commented Apr 9, 2020

@shahaf-sameach Thanks for the offer. Yes, please give that a try and let us know how it goes.

@snicoll
Copy link
Member

snicoll commented Apr 17, 2020

I think going forward we should also make sure to add @since on the deprecation message, see #20991.

@shahaf-sameach how is it going by the way? Do you need any help?

@philwebb
Copy link
Member

We could add a since attribute on @DeprecatedConfigurationProperty

@shahaf-sameach
Copy link

hi @snicoll, thank you, just wanted to ask if that is straight forward as it seems, look for any @Deprecated occurrence at a getter, and check the setter also annotated, am I missing something?

@snicoll
Copy link
Member

snicoll commented Apr 18, 2020

Yes.

@snicoll
Copy link
Member

snicoll commented Apr 19, 2020

@shahaf-sameach and I discussed offline and we've decided I would take over this one after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

6 participants