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.

Sign up for GitHub

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

Jump to bottom

Deprecate fields that are derived from containerInfo #1585

Merged
merged 6 commits into from
Jul 6, 2019
Merged

Deprecate fields that are derived from containerInfo #1585

merged 6 commits into from
Jul 6, 2019

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Jul 2, 2019

No description provided.

@bsideup bsideup added this to the next milestone Jul 2, 2019
@bsideup bsideup requested a review from a team July 2, 2019 11:07
kiview
kiview reviewed Jul 2, 2019
View reviewed changes
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

Is the PR in a valid state now?

core/src/main/java/org/testcontainers/containers/GenericContainer.java
@@ -188,6 +197,10 @@ public GenericContainer(@NonNull final Future<String> image) {
this.image = image;
}

public String getContainerId() {
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference to the Lombok getter?

Copy link
Member Author

Choose a reason for hiding this comment

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

not deprecated

@bsideup
Copy link
Member Author

bsideup commented Jul 2, 2019

@kiview yes. I reverted a change to getContainerId because we have some usages of containerId before we query containerInfo.

We will make the field private after the deprecation period, so that only GenericContainer will be able to use it, and others will have to use getContainerId that delegates to containerInfo?.getId()

@bsideup bsideup requested review from rnorth and kiview July 5, 2019 09:38
@rnorth rnorth merged commit 06ea95b into master Jul 6, 2019
@delete-merged-branch delete-merged-branch bot deleted the deprecate_fields_derived_from_containerInfo branch July 6, 2019 21:18
@rnorth
Copy link
Member

rnorth commented Jul 7, 2019

This was released in 1.11.4 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnorth rnorth rnorth approved these changes

@kiview kiview Awaiting requested review from kiview

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.11.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bsideup @rnorth @kiview