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

Break up quickstart example projects and introduce JUnit 5 refe… #1416

Merged

Conversation

bmuschko
Copy link
Contributor

This change adds a quickstart reference for using TestContainers with JUnit 5. Still uses the "old" Gradle configurations to ensure uniformity across the whole documentation.

@kiview
Copy link
Member

kiview commented Apr 24, 2019

Thanks, this looks great!
Just one thing, we kind of decided to go on moving all code examples (also the doc ones) to ./examples in the project root (@bsideup @rnorth please correct me if I'm wrong).

Would you be okay with doing it for the examples that correspond to this PR?

@bmuschko
Copy link
Contributor Author

Would you be okay with doing it for the examples that correspond to this PR?

I think it would be more straight forward to keep the structure you have right now. How about merging this PR first? I'd be happy to follow up with another PR shortly thereafter that moves the examples in general. This approach would lead to less, better reviewable changes per PR as all references to example would have to be changed.

@bmuschko
Copy link
Contributor Author

@kiview Any more thoughts on this? Is there anything else I can do to get the changes merged?

@bmuschko
Copy link
Contributor Author

Is this still of interest? Otherwise I might as well close it.

@rnorth
Copy link
Member

rnorth commented May 26, 2019

Sorry @bmuschko this is still of interest - we've just all been quite snowed under for several weeks. I'll try and review/merge this week.

I'm very sorry for the delay.

Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

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

I think this is fine apart from a couple of very minor points.

Directory structure-wise we have to acknowledge that this isn't the final state. It's OK to get the content in and then refactor.

I think I'll want to do another bulk move of content once this and #1499 are merged.

compile "io.lettuce:lettuce-core:5.1.1.RELEASE"

def junitJupiterVersion = '5.4.2'
testCompile "org.junit.jupiter:junit-jupiter-api:$junitJupiterVersion"
Copy link
Member

Choose a reason for hiding this comment

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

Please could you inline the versions and remove the variable? Even though it causes duplication, Dependabot can then keep the versions up to date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

docs/quickstart/junit_5_quickstart.md Show resolved Hide resolved
Dependabot needs to be able to discover those and propose a change.
@stale
Copy link

stale bot commented Aug 26, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@bsideup
Copy link
Member

bsideup commented Sep 1, 2019

@rnorth I volunteer to deliver this PR.

Could you please re-review (since @bmuschko applied some changes)?

@bsideup bsideup requested a review from rnorth September 1, 2019 07:41
@rnorth
Copy link
Member

rnorth commented Sep 1, 2019

@bsideup please could you resolve the merge conflicts, then I think it's basically good to go.

It would be good to have another go at making the docs codeinclude the dependencies directly from the relevant build.gradle file, so that the docs effectively get updated by dependabot when new versions of JUnit 5 are released.

We could do that later though; it'd be good to get this merged even without that.

@bsideup bsideup changed the base branch from master to junit5-quickstart November 18, 2019 07:22
@bsideup bsideup added this to the next milestone Nov 18, 2019
@bsideup
Copy link
Member

bsideup commented Nov 22, 2019

@rnorth resolved

@rnorth
Copy link
Member

rnorth commented Nov 23, 2019

Awesome, thanks @bsideup.

@bmuschko thank you so much for this PR, and sorry that it's taken such a long time!

Merging 😄

@rnorth rnorth changed the title Break up quickstart example projects and introduce JUnit 5 reference Break up quickstart example projects and introduce JUnit 5 refe… Nov 23, 2019
@rnorth rnorth merged commit 2bde224 into testcontainers:junit5-quickstart Nov 23, 2019
rnorth added a commit that referenced this pull request Nov 27, 2019
* Break up quickstart example projects and introduce JUnit 5 refe… (#1416)

* Break up quickstart example projects and introduce JUnit 5 reference

* Use for dependency declarations

Dependabot needs to be able to discover those and propose a change.

* Relocate Spock example code


Co-authored-by: Benjamin Muschko <benjamin.muschko@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants