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

drop unused constructors #5941

Closed
wants to merge 3 commits into from

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented Apr 23, 2024

No description provided.

@@ -34,10 +34,6 @@ public ConfigMapLock(String configMapNamespace, String configMapName, String ide
super(configMapNamespace, configMapName, identity);
}

public ConfigMapLock(ObjectMeta meta, String identity) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these two constructors does not seem to be used, so I decided to drop them. This will be a breaking change if the next release is not a major one, though. I am not sure what is the policy in this project about that.

Copy link
Member

Choose a reason for hiding this comment

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

Usually we deprecate stuff like this first in next minor release and then remove it in follow up releases. If this class is directly exposed to users of KubernetesClient API, we would want to deprecate it first, add required JavaDocs and notice about deprecation in CHANGELOG.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

understood, we kind of do the same thing in spring... I'll change the code in that regard. thank you

Copy link
Member

Choose a reason for hiding this comment

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

these two constructors does not seem to be used

This is part of a public facing API. How are you aware of the lack of usage of these constructors on downstream lock suppliers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, I'm not. The thing is, on some other issue a while back, we talked about how breaking changes are somehow OK in this project, since k8s does not maintain backwards compatibility...

But as said, I'll listen to your advices, since I don't know how things are handled here. In spring, we deprecate, and remove in the next major release.

@wind57 wind57 changed the title drop constructors drop unused constructors Apr 23, 2024
Copy link

sonarcloud bot commented Apr 25, 2024

@wind57 wind57 closed this Apr 29, 2024
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.

None yet

3 participants