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

Update to Gradle 7 #4171

Merged
merged 9 commits into from
Jun 9, 2021
Merged

Update to Gradle 7 #4171

merged 9 commits into from
Jun 9, 2021

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Jun 6, 2021

No description provided.

@bsideup bsideup added this to the next milestone Jun 6, 2021
@bsideup bsideup added area/shading dependencies Pull requests that update a dependency file gradle-wrapper Pull requests that update Gradle wrapper type/housekeeping labels Jun 6, 2021
@bsideup bsideup marked this pull request as ready for review June 6, 2021 22:28
for (dependency in rootNode.dependencies.children()) {
def coordinates = "${dependency.groupId.text()}:${dependency.artifactId.text()}".toString()
if (!dependencies.contains(coordinates) && !whitelist.contains(coordinates)) {
throw new IllegalStateException("New dependency! ${coordinates}")
Copy link
Member

Choose a reason for hiding this comment

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

Could we make this exception message more helpful for contributors?

e.g.

Suggested change
throw new IllegalStateException("New dependency! ${coordinates}")
throw new IllegalStateException("A new dependency has been added ${coordinates} - if this was intentional please add it to the whitelist in PATH_TO_GRADLE_FILE")

What do we expect the process to be - do we need to be approving new dependencies somehow?

BTW I think it would be good to move this to a separate .gradle file, so that areas of major scripting are encapsulated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! We could, yes 👍
FYI this message is mostly for us, maintainers, as the chance of an external dependency added by external contributors is super low

Copy link
Member Author

Choose a reason for hiding this comment

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

Extracted and made the message more user friendly 👍

@@ -1,16 +1,16 @@
description = "Testcontainers :: Selenium"

dependencies {
compile project(':testcontainers')
api project(':testcontainers')

provided 'org.seleniumhq.selenium:selenium-remote-driver:3.141.59'
Copy link
Member

Choose a reason for hiding this comment

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

BTW I've verified that the resulting POM files are effectively the same, using the approach described here, so I'm happy with this.

# Conflicts:
#	modules/database-commons/build.gradle
#	modules/dynalite/build.gradle
#	modules/elasticsearch/build.gradle
#	modules/jdbc/build.gradle
#	modules/junit-jupiter/build.gradle
#	modules/kafka/build.gradle
#	modules/localstack/build.gradle
#	modules/nginx/build.gradle
#	modules/vault/build.gradle
for (dependency in rootNode.dependencies.children()) {
def coordinates = "${dependency.groupId.text()}:${dependency.artifactId.text()}".toString()
if (!dependencies.contains(coordinates) && !ignore.contains(coordinates)) {
throw new IllegalStateException("A new dependency '${coordinates}' has been added to 'org.testcontainers:${artifactId}' - if this was intentional please add it to the ignore list in ${project.buildFile}")
Copy link
Member

@rnorth rnorth Jun 9, 2021

Choose a reason for hiding this comment

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

ignore list

👏

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.

Looks Awesome To Me 😄

@bsideup bsideup merged commit 9a8df79 into master Jun 9, 2021
@delete-merged-branch delete-merged-branch bot deleted the gradle_7 branch June 9, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/shading dependencies Pull requests that update a dependency file gradle-wrapper Pull requests that update Gradle wrapper type/housekeeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants