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 pluginManagement documentation to mention global configuration options #4999

Merged
merged 7 commits into from Apr 20, 2018

Conversation

Projects
None yet
2 participants
@apottere
Contributor

apottere commented Apr 10, 2018

#4920

Contributor Checklist

  • Review Contribution Guidelines
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:check

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation
  • Recognize contributor in release notes
Update pluginManagement documentation to mention global configuration…
… options

Issue: #4920

Signed-off-by: Andrew Potter <apottere@gmail.com>
@apottere

This comment has been minimized.

Contributor

apottere commented Apr 10, 2018

I wasn't sure how to format the code snippets filenames since they weren't included from source files, so I put their name in bold above the snippet. Let me know if that should be changed.

@apottere

This comment has been minimized.

Contributor

apottere commented Apr 10, 2018

I'm also not sure if the link I added is working correctly, I attempted to copy the format of the other local links in the file.

@oehme oehme self-requested a review Apr 10, 2018

@oehme oehme self-assigned this Apr 10, 2018

@oehme oehme added this to the 4.8 RC1 milestone Apr 10, 2018

@oehme

oehme approved these changes Apr 17, 2018

This looks good, I just had a small stylistic comment. Would you mind addressing that before I merge?

[source,groovy]
----
settingsEvaluated { settings ->
settings.pluginManagement.resolutionStrategy {

This comment has been minimized.

@oehme

oehme Apr 17, 2018

Member

Can you change this to just one block like the example above? That would look just a bit more idiomatic. :)

apottere added some commits Apr 17, 2018

Move code snippets into samples instead.
Issue: #4920

Signed-off-by: Andrew Potter <apottere@gmail.com>
@apottere

This comment has been minimized.

Contributor

apottere commented Apr 17, 2018

@oehme I converted the blocks to snippets, and I've verified they get copied to the build/samples directory when running :docs:userguide. Does that address your comment or was there something else I need to change also?

@oehme

This comment has been minimized.

Member

oehme commented Apr 17, 2018

That's not what I meant, having them inline was just fine. I meant doing

settings.pluginManagement {
  repositories {
  }
  resolutionStrategy {
  }
}

instead of

settings.pluginManagement.repositories {

}

settings.pluginManagement.resolutionStrategy {

}
Collapse empty lines in blocks
Issue: #4920

Signed-off-by: Andrew Potter <apottere@gmail.com>
@apottere

This comment has been minimized.

Contributor

apottere commented Apr 18, 2018

@oehme ah, well in that case I updated the snippets to remove empty lines - I wanted to make them snippets to match the other files in the docs anyway.

oehme added some commits Apr 20, 2018

@oehme oehme merged commit eac772d into gradle:master Apr 20, 2018

2 checks passed

DCO All commits have a DCO sign-off from the author
Details
WIP ready for review
Details
@oehme

This comment has been minimized.

Member

oehme commented Apr 20, 2018

Thank you @apottere!

@apottere apottere deleted the apottere:doc-update branch Apr 21, 2018

@apottere

This comment has been minimized.

Contributor

apottere commented Apr 21, 2018

@oehme NP! I noticed this commit (069f30f) you added before pulling - did you test those changes? In 4.6 at least the block can't be written that way - the only way to call those specific blocks in an init script is to call

settings.pluginManagement.resolutionStrategy { }

as opposed to

settings.pluginManagement {
    resolutionStrategy {
    }
}
@oehme

This comment has been minimized.

Member

oehme commented Apr 22, 2018

It works just fine for me. It's calling the exact same method so any difference would be very surprising. Can you check again?

@apottere

This comment has been minimized.

Contributor

apottere commented Apr 24, 2018

@oehme Sorry, I guess I was mistaken - it does work both ways. I don't remember why I thought it didn't.

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