Closed whikloj closed 5 years ago
Merging #55 into master will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #55 +/- ##
=========================================
Coverage 80.36% 80.36%
Complexity 93 93
=========================================
Files 17 17
Lines 331 331
Branches 1 1
=========================================
Hits 266 266
Misses 64 64
Partials 1 1
Continue to review full report at Codecov.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update 3b5af5b...1ce5ba6. Read the comment docs.
I just did a similar thing on the Trellis repos just last week. It looks like this configuration will work fine, but there are two considerations for things to add.
One is that, when it comes time to release the artifacts, you might not want the automated build of the release commit to interfere with the locally-initiated ./gradlew release
process (one set of artifacts will be signed, and therefore valid for Maven central, while the other set won't be signed). I found it easiest to add a getVersion
task in the build.gradle file and add a filter to the script
stage.
The other consideration is that, periodically, you may find that pushing the build to Sonatype will fail -- there may be weird, transient network issues. Given that, the entire build will fail with this configuration. If that's what you want, then leave this as it is. My perspective is that the tests need to pass, but pushing to Sonatype (and Docker) is just a convenience, so I allow those to fail (even though they succeed 95% of the time).
BTW, I trust that the encrypted username/password here are revokable. That is, Sonatype will provide a revokable username/password token for a given user so that you don't need to provide Travis-CI with a real username/password combination. I have also found that putting these values in environmental variables might be somewhat easier -- if you need or want to rotate your keys every N months, you won't need to send a new commit just for that purpose: you can just change the settings on the Travis build directly.
I'm not sure about the release versus snapshot release issue, I'll defer to @jonathangreen and @dannylamb on that one.
I did use my account information, so thank you for that. I'll swap it out but also see about adding it to the Travis setup instead. π
Hmm to use encrypted environment variables, the PRs have to come from the same repo. So this will not deploy anything when the PR is from a forked repository. https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
Not that I consider this a problem, just something to be aware of.
Though you wouldn't want the pull-request to trigger a deployment, correct? Only after the PR is merged should the deploy
stage factor in at all.
@acoburn I'm not really clear how we were using these snapshot deployments in the first place, so it is possible this is how it was working before. In which case π we're good π
@whikloj I agree. I think this looks good.
My understanding was the deploy stage isn't triggered on pull request builds, only after they are merged. That's a Travis thing I should probably confirm.
PR is good and I'm pulling it in. Thx @whikloj and @acoburn.
GitHub Issue: N/A
What does this Pull Request do?
Updates the sonatype environment variables and (hopefully) allows us to release the snapshot builds.
What's new?
How should this be tested?
Travis is
Interested parties
@Islandora-CLAW/committers