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

Moving gradle wrapper to default location #200

Merged
merged 4 commits into from Mar 29, 2020

Conversation

Bukama
Copy link
Member

@Bukama Bukama commented Mar 27, 2020

Title says all ;)

With this location IntelliJ could determine the correct version automaticall with both settings (using wrapper.properties and executing warpper-task) which was not the case before the change.

fixes #190


I hereby agree to the terms of the JUnit Pioneer Contributor License Agreement.

Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

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

The change is correct, but as it is the default value, there is no reason to define this, i think we are save with removing the 3 lines alltogether. The gradle wrapper task is a default gradle task, we only need to define the block when we change something

build.gradle.kts Outdated
@@ -184,7 +184,7 @@ publishing {
}

tasks.wrapper {
jarFile = file(".infra/gradle/gradle-wrapper.jar")
jarFile = file("gradle/wrapper/gradle-wrapper.jar")
Copy link
Member

Choose a reason for hiding this comment

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

i think this is the default so you actually do not need to set it at all - actually we do not need the whole block, when we use default settings

Copy link
Member Author

Choose a reason for hiding this comment

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

i think this is the default so you actually do not need to set it at all - actually we do not need the whole block, when we use default settings

I don't know gradle very well but maybe this is needed for the option to use the gradle setting via a wrapper task. At least IntelliJ offers such an option beside use default wrapper.properties location and use own location.

Copy link
Member

Choose a reason for hiding this comment

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

https://docs.gradle.org/current/dsl/org.gradle.api.tasks.wrapper.Wrapper.html#org.gradle.api.tasks.wrapper.Wrapper:jarFile

this is the default value, and the gradle wrapper task is something, which gradle provides out of the box since verison 5 i think (do not pin me on that). but you can find that information:

Every vanilla Gradle build comes with a built-in task called wrapper. You’ll be able to find the task listed under the group "Build Setup tasks" when listing the tasks. Executing the wrapper task generates the necessary Wrapper files in the project directory. - https://docs.gradle.org/current/userguide/gradle_wrapper.html#sec:adding_wrapper

To test this, you can simply remove the block and run gradle wrapper in your cli and see, it will work out of the box

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok removed the block and still seems to work. Thanks for the docs!

@aepfli
Copy link
Member

aepfli commented Mar 29, 2020

normally there should be also a change in the gradlew and gradlew.bat as the path has changed, and there is a different path - i am currently not sure. but we need to check those files before merging

Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

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

@nipafx nipafx changed the title Movig gradle wrapper to default location Moving gradle wrapper to default location Mar 29, 2020
@Bukama
Copy link
Member Author

Bukama commented Mar 29, 2020

regenerate gradlew and gradlew.bat with gradle wrapper as they are pointing currently to the .infra folder

https://github.com/Bukama/junit-pioneer/blob/issue190/movegradleconfig/gradlew#L83

https://github.com/Bukama/junit-pioneer/blob/issue190/movegradleconfig/gradlew.bat#L82

Done. Thanks again

@Bukama Bukama requested a review from aepfli March 29, 2020 12:53
@nipafx
Copy link
Member

nipafx commented Mar 29, 2020

Rebased on master to include the 6.3 update. Seems to work, thanks @Bukama and @aepfli. 👍

@nipafx nipafx merged commit 0150796 into junit-pioneer:master Mar 29, 2020
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.

Config files are not discovered by tools
3 participants