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

Add missing @Deprecated annotations #7895

Closed
wants to merge 1 commit into from

Conversation

perlun
Copy link
Contributor

@perlun perlun commented Nov 29, 2023

This should have been included in #7652, but was likely forgotten.

This should have been included in testcontainers#7652,
but was likely forgotten.
@perlun perlun requested a review from a team as a code owner November 29, 2023 13:42
@eddumelendez
Copy link
Member

Hi, thanks for the PR. The story for GenericContainer is going to be different than Container. That's why it has been only deprecated in one place for now.

@perlun perlun deleted the patch-1 branch November 30, 2023 10:52
@perlun perlun restored the patch-1 branch November 30, 2023 10:58
@perlun
Copy link
Contributor Author

perlun commented Nov 30, 2023

I see @eddumelendez, thanks for the clarification. Will this not be a bit inconsistent though? For child classes extending GenericContainer, they will have the following set of methods (this is from an example class in our code base which extends FixedHostPortGenericContainer<HiboxcentreTomcatContainer>):

image

Feel free to disagree, but I believe the above API becomes a bit confusing to the user. 🤔 Some withFileSystemBind() methods are fine to use but not others. At the very least, we should then try to document this better so that it's clear for the end user of Testcontainers what to expect.

/cc @kiview FYI, since you were involved in the original PR (#7652).

@big-andy-coates
Copy link
Contributor

Aren't the methods in GenericContainer implementations of the deprecated methods in Container, and as such would inherit the @Deprecated annotation?

@perlun
Copy link
Contributor Author

perlun commented Jan 16, 2024

@big-andy-coates From how I understand it: no, the annotations aren't inherited that way. Since GenericContainer overrides these methods from the Container interface, the annotations need to be duplicated in GenericContainer (if the methods are to be deprecated, which @eddumelendez indicated that they won't be). More details: https://stackoverflow.com/a/10082663/227779

@perlun perlun deleted the patch-1 branch January 16, 2024 11:46
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