-
Notifications
You must be signed in to change notification settings - Fork 214
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
Publish Gradle build scans from CI #1481
Conversation
if (System.getenv('CI') == 'true') { | ||
tag 'CI' | ||
} else { | ||
publishing.onlyIf { false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this setup disable local ad-hoc --scan
usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tried it, copied from cashapp/molecule. The intent is for CI build scans only though, if that's the root question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intent is for CI build scans only though, if that's the root question.
Yep, I get what the goal is. I'm just highlighting that the current behaviour of gradlew --scan
is going to be disabled locally. It's a surprise for any contributor.
If anyone wants to debug they'll have to first find this setting why it's not working and the move on to debugging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just highlighting that the current behaviour of
gradlew --scan
is going to be disabled locally. It's a surprise for any contributor.If anyone wants to debug they'll have to first find this setting why it's not working and the move on to debugging.
Out of curiosity, have you verified this? Because when I run gw paparazzi:test --scan
from this branch, the output included a link to this build scan, so I think your concerns are covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because you have CI set, try it without.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that's my bad then it seems --scan
is the winner whatever is in the build script:
https://docs.gradle.com/develocity/gradle-plugin/current/#using_build_scans
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However #1481 (comment) does matter now, because --scan
will auto-publish even locally without asking for an acceptance. Committing this outside of the CI block means you accept the terms on behalf of all the contributors.
See the warning at https://docs.gradle.com/develocity/gradle-plugin/current/#connecting_to_scans_gradle_com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've read the documented Gradle warning. Thanks for providing that.
But I'm still confused...what is the root concern here? If a project accepts the terms and a contributor subsequently passes the --scan
option, wouldn't that imply that the terms would be accepted?
That being said, I'm not a lawyer and don't have a strong opinion either, so I've updated the PR to reflect your concern.
I would also be curious to get @JakeWharton's input here, since a quick search of Square and CashApp OSS repos show that they're similarly configured, which is why I went with that:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a project accepts the terms and a contributor subsequently passes the --scan option, wouldn't that imply that the terms would be accepted?
Yes that's true, but as far as I understand "the project" is not really a legal entity, so you as the maintainer cannot accept the terms on behalf a contributor. It's them that use the services (of Gradle), when they publish their personal environment data from their computer, not "the project". Not a lawyer either, this is just my understanding. Individuals can accept the terms globally in their init scripts on each machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone did it once (purportedly incorrectly) and it was copy/pasted a million times. You were just doing your duty by copy/pasting it further.
Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
9836d98
to
b075f29
Compare
To help investigate Gradle caching issues