Skip to content

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
@bsideup bsideup requested review from kiview and rnorth as code owners 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 👍

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.

bsideup added 3 commits June 9, 2021 17:05
# 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.

3 participants