WIP - Add Shipkit for release management #549

jpkrohling · 2018-09-14T09:27:16Z

Signed-off-by: Juraci Paixão Kröhling [email protected]

Which problem is this PR solving?

All our release problems :-)
Resolves Move to bintray #536

Short description of the changes

Removed everything related to the way we did releases before
Added Shipkit and all it needs to do releases now

yurishkuro · 2018-09-14T14:17:03Z

RELEASE.md

+1. Update the `CHANGELOG.md` with the changes for the last release. Most of the changes should have been added there already, but at least the release date has to be set.
+1. Check the `version.properties` file: the `version` property is the version that is going to be released next. If you intend to have a different version, update this file.
+1. Get the changes above merged
+1. Tag the release, like, `git tag release/v0.31.1` and push the tag to the main repository (`git push upstream v0.31.0`, if your `jaegertracing/jaeger-client-java` remote is called `upstream`)


Can we instead do a release on GitHub, with proper release notes?

This tag is not the release itself, it's the tag to tell Travis to release (similar to the release-v... that we have for OT projects).

GitHub releases is something we'll need to continue doing manually for now, until mockito/shipkit#75 is done.

There is a missing step to create release on github. Tag does not create releae on github.

From my previous comment:

GitHub releases is something we'll need to continue doing manually for now, until mockito/shipkit#75 is done.

By the way: as we have a GitHub token, we can certainly automate this in a follow up PR, probably by doing some REST calls in the release.sh after the Shipkit release finishes.

yurishkuro · 2018-09-14T14:19:52Z

build.gradle

@@ -76,19 +70,6 @@ subprojects {
        from sourceSets.main.output
    }

-    if (!version.endsWith("SNAPSHOT")) {


Does it mean we're not going to do snapshot releases, or is it done elsewhere? If the latter, would be good mention in the README.

We will not be having snapshots anymore.

@jpkrohling what do you mean by not having snapshots? Is a not released version going to compile as a.b.c ?

Yes, but there will not be any 0.31.2-SNAPSHOT published to any public repository.

Is it skipKit limitation or just the way how it's done here?

I would like to keep snapshot builds. If we go away form snapshots we will have to remove version from local repo to include released changes. It's not very convenient.

Is it skipKit limitation or just the way how it's done here?

Snapshots are a Maven thing.

I would like to keep snapshot builds. If we go away form snapshots we will have to remove version from local repo to include released changes. It's not very convenient.

Looks like you don't want snapshots. Looks like what you don't want is to have artifacts installed in ~/.m2/repo whenever you do a build. You can skip it by passing -x publishToMavenLocal. Or we can add publishToMavenLocal.enabled = false to build.gradle.

If you really, really want snapshots, I'm open to hear your arguments.

I want snapshots to build and installed to my local repo. I use it often when working on different components which include jaeger.

I think I already mentioned my arguments. There are plenty of arguments out there why to use snapshots.

I use it often when working on different components which include jaeger.

I considered this case as well, but in the end, I think my workflow will barely change. I can always update the version.properties to include -SNAPSHOT in the version string.

The advantage of dropping snapshots altogether is that there's no logic needed to handle this scenario during the releases, so, we need only one version change, instead of two (0.0.1-SNAPSHOT -> 0.0.1 -> 0.0.2-SNAPSHOT).

A positive side-effect, IMO, is that I can test the real version string on PRs that I prepare before the release is done, like I did for the SmallRye PR: smallrye/smallrye-opentracing#25 . The branch was tested and ready to be sent as PR before the release reached Maven Central.

The snapshot is there for some reasons. For instance avoid using local changes where you expect released version. If it's dropped it's hard to know what version of client is being used in your local development.

A positive side-effect, IMO, is that I can test the real version string on PRs that I prepare before the release is done, like I did for the SmallRye PR: smallrye/smallrye-opentracing#25 . The branch was tested and ready to be sent as PR before the release reached Maven Central.

Or you could have done changes which you forgot that you made and the change to smallrye would not work bc you expect something else in the repo :)

How about we add publishToMavenLocal.enabled = false to build.gradle and leave snapshots out of this PR, at least at the beginning? Our use case doesn't apply to everyone and it doesn't happen everyday, so, I'd rather have the release script and the git history simplified. Then, when we need to consume a non-published artifact (snapshot, in maven terms), we run ./gradlew snapshot.

If it turns out that we can't live without snapshots in the version.properties (like everything other than maven can), than we reintroduce it in the release script, as an additional commit.

yurishkuro · 2018-09-14T14:23:42Z

version.properties

@@ -0,0 +1,2 @@
+version=0.31.1
+previousVersion=0.31.0


What is it used for? I dislike manual maintenance.

That's bumped automatically by Shipkit. We only touch it when we decide to manually release or do a bigger bump (v0.32.0, for instance). But see next comment.

There's no need anymore to touch this file. Tagging + make release will bump it automatically, be it via Travis, be it in a manual release.

yurishkuro · 2018-09-14T14:25:21Z

RELEASE.md

+## Automated
+
+1. Update the `CHANGELOG.md` with the changes for the last release. Most of the changes should have been added there already, but at least the release date has to be set.
+1. Check the `version.properties` file: the `version` property is the version that is going to be released next. If you intend to have a different version, update this file.


What happens if the version in the tag is different from this file?

That's a great question. I actually thought about having a step in the makefile to check the branch name and update the version.properties accordingly, but forgot to add to the PR.

jpkrohling · 2018-09-17T16:03:33Z

I'm adding a WIP tag, as I need to check the generated POM for Maven Central requirements. Other than that, this is ready to be re-reviewed.

pavolloffay · 2018-09-18T08:52:22Z

gradle/shipkit.gradle

+    // https://github.com/orgs/jaegertracing/teams/jaeger-maintainers/members
+    // sorted alphabetically by username
+    team.developers = [
+            'black-adder:Won Jun Jang',


Where is this being used?

Could we just point it to https://gitter.im/jaegertracing/Lobby?

No, it's required by Maven Central. It's used in pom.xml that is generated as part of the published artifacts.

There is no need to include everybody. Some OSS projects use link to forum or chat to avoid including all people. I guess we could use gitter: https://gitter.im/jaegertracing/Lobby

https://maven.apache.org/pom.html#Developers

https://github.com/opentracing/opentracing-java/blob/master/pom.xml#L94

I'd rather not have this there, but if it's required, then let's please have the appropriate data there, not just some data to please Central...

i don't mind it was just a suggestion. It also makes it simpler to get in touch

A downside of listing all the core developers there, is that this has the potential to get outdated easily. Let's try to get the right thing first and, if it does get outdated often, we can consider adding some random data (like the gitter channel).

pavolloffay · 2018-09-18T08:52:57Z

gradle/shipkit.gradle

@@ -0,0 +1,40 @@
+shipkit {


Shouln'd there be a license setting?

No, not for the Shipkit configuration, but there's one a few lines below for Bintray.

jpkrohling · 2018-09-19T07:10:19Z

I'm marking this as WIP, as it's blocked by mockito/shipkit#755

codecov · 2018-09-19T08:07:41Z

Codecov Report

Merging #549 into master will decrease coverage by 0.26%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #549      +/-   ##
============================================
- Coverage     89.12%   88.86%   -0.27%     
+ Complexity      521      519       -2     
============================================
  Files            67       67              
  Lines          1895     1895              
  Branches        243      243              
============================================
- Hits           1689     1684       -5     
- Misses          133      137       +4     
- Partials         73       74       +1

Impacted Files	Coverage Δ	Complexity Δ
...rtracing/internal/reporters/CompositeReporter.java	`71.42% <0%> (-28.58%)`	`6% <0%> (-1%)`
...gertracing/internal/reporters/LoggingReporter.java	`81.81% <0%> (-9.1%)`	`4% <0%> (-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 6ee913a...cdbe0f5. Read the comment docs.

pavolloffay · 2018-09-19T08:12:27Z

RELEASE.md

+
+1. Update the `CHANGELOG.md` with the changes for the last release. Most of the changes should have been added there already, but at least the release date has to be set.
+1. Get the change above merged
+1. Tag the release, like, `git tag release/v0.31.1` and push the tag to the main repository (`git push upstream v0.31.0`, if your `jaegertracing/jaeger-client-java` remote is called `upstream`)


Could we keep the same tag names? So without release/?

If there are intermediate commits required, like when bumping from 0.0.2 to 1.0.0, then we should do it in the release/ tag build, otherwise our v1.0.0 tag would have version.properties as 0.0.2.

I am not sure if I follow you. Does the release process crate a tag? So when I push release/vx.x.x does it create vx.x.x?

Could you please explain it for both cases what tags are created when there is a change and when there is no change required in version file?

Does the release process crate a tag? So when I push release/vx.x.x does it create vx.x.x?

Yes, when you push release/v0.0.1, it creates the v0.0.1 tag.

Could you please explain it for both cases what tags are created when there is a change and when there is no change required in version file?

Sure, I'll update the readme.

Readme updated

jpkrohling · 2018-09-19T08:37:41Z

So, looks like the only point left to discuss is about having or not having -SNAPSHOT in the version.properties . Currently, we have two options:

Single-commit releases: 0.0.1 -> 0.0.2
Two-commit releases: 0.0.1-SNAPSHOT -> 0.0.1 -> 0.0.2-SNAPSHOT

This PR currently adopts the first approach. For local development, the same effect as the second can be achieved by running ./gradlew snapshot instead of ./gradlew build.

Would you please vote for your desired approach?

cc @pavolloffay, @objectiser, @yurishkuro, @vprithvi, @black-adder

objectiser · 2018-09-19T08:49:41Z

My current preference is (1) as it keeps a cleaner commit history and does not prevent us from creating snapshots in the local maven repo - just means we need to do it explicitly.

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

jpkrohling · 2018-09-25T11:59:48Z

@yurishkuro, @vprithvi, @black-adder , any comments about the single commit release vs. two-commit releases?

vprithvi · 2018-09-25T14:45:12Z

@jpkrohling Could you elaborate on 1?

Lets say I have changes a, b, c:

Would merging these to master look like the following:

merge a -> automatically release version 0.0.1
merge b -> automatically release version 0.0.2
merge c -> automatically release version 0.0.3

If so, this brings in a semantic change in that all merges to master have a PATCH release, and that we need to guarantee that they are working (As opposed to SNAPSHOT releases, which have no expectations of being ready to use).

jpkrohling · 2018-09-25T15:15:47Z

@vprithvi that's how Shipkit works out of the box, but it's not what we'll be doing for the Jaeger Java Client, at least for now.

Instead of doing continuous delivery (which is what you exemplified), we'll be tagging releases explicitly, like git tag release/v0.40.0. Travis will pick this new tag up and prepare the release v0.40.0, which results in a tag of the same name as the version.

What this Travis build does is what we are discussing: if we always have a "snapshot" version on version.properties, the tagged version needs to happen on an intermediary commit (0.40.1-SNAPSHOT -> 0.40.1 -> 0.40.2-SNAPSHOT). Otherwise, the tag happens on whatever lands on master (0.40.1 -> 0.40.2).

pavolloffay · 2018-09-25T15:33:11Z

@vprithvi @yurishkuro @black-adder

The main question here is whether we keep -SNAPSHOT in version for a not released code or we get rid of it.

If we get rid of it there will be one less commit in the history while releasing. I personally don't mind two commits it's what maven does by default. The other thing to be careful about is that removing snapshot can result in unpredictable local builds/runtime for packages using jaeger client as a dependency. For example jaeger-core version 0.31.0 could represent your old local experimental branch and not a released version.

jpkrohling · 2018-09-25T15:38:51Z

jaeger-core version 0.31.0 could represent your old local experimental branch

That's true. To help prevent that, we can disable the publishing by default, so that you'd need to run ./gradlew install to get a build installed on the local maven repo. AFAIK, the current state is that ./gradlew build will install the built artifact into the local maven repo.

pavolloffay · 2018-09-25T15:44:06Z

build target does not install to local repo. Only install target installs it to local repo. This is what gradle does by default it mimics maven - package -> install.

vprithvi · 2018-09-25T15:51:23Z

@jpkrohling @pavolloffay Thanks for clarifying!

To me the question boils down to whether we want to have intermediate artifacts published. If we do, I prefer using the SNAPSHOT mechanism because it is familiar and because the artifacts go to a separate snapshot repository. Additionally, a lot of Java tooling exists for consuming and working with snapshot artifacts.

I've not yet worked with projects that increase patch versions without doing CD. While it seems simpler, I think it makes it difficult to differentiate between intermediate artifacts and releases. Also, where are these intermediate artifacts published?

jpkrohling · 2018-09-25T16:00:33Z

Additionally, a lot of Java tooling exists for consuming and working with snapshot artifacts.

My impression is that nobody consumes snapshots published at maven central, as they are unpredictable. Someone who did a build one hour ago might have a different build than someone building it now, due to how caching works for snapshots. Snapshots are usually only useful locally, when we are building a patch for one project and testing in another. For that scenario, we have ./gradlew snapshot.

I've not yet worked with projects that increase patch versions without doing CD

Increasing the patch version was only an example. What controls the version that is going to be released is the tag that you create manually: git tag release/v0.32.0 will release 0.32.0, no matter what's the last released version.

Also, where are these intermediate artifacts published?

Perhaps it's implicit on my previous comment, but there's no intermediary artifact, only released artifacts. They end up eventually on Maven Central, via Bintray.

yurishkuro · 2018-09-25T16:49:26Z

In the other repos (go, Python, node) we are using two-commit style and those are done manually, which is a bit annoying. But the two commits themselves sound good to me.

jpkrohling · 2018-09-27T10:41:42Z

My impression is that nobody consumes snapshots published at maven central

~~It looks like no OpenTracing project is publishing snapshots as well: https://oss.sonatype.org/content/repositories/snapshots/io/opentracing/~~

Spoke too soon: looks like they are published, but at JFrog's snapshot repository, which doesn't seem to be synchronized to Central: https://oss.jfrog.org/artifactory/oss-snapshot-local/io/opentracing/

pavolloffay · 2018-10-15T11:40:01Z

We have talked about this internally. If we don't need any of features introduced by this we try to fix the current release process and continue using that. Comment if you need something introduced by in this PR. @yurishkuro @vprithvi

yurishkuro · 2018-10-15T13:50:26Z

To me the main question is whether it publishes the release artifacts to maven Central and whether it is reliable. If yes then I think it's worth making the change, because the experience with the current release plugin is just miserable.

pavolloffay · 2018-10-15T14:20:32Z

The problem with the current release is because travis is changing public IP which breaks upload process. There are also other workarounds: migrate to circle-ci, use vpn for the upload.

jpkrohling · 2018-10-15T15:24:48Z

To me the main question is whether it publishes the release artifacts to maven Central and whether it is reliable

So far, yes. I used a couple of times for https://github.com/opentracing-contrib/java-interceptors and it does seem to work consistently. The only blocking issue I have is mockito/shipkit#755 .

There are also other workarounds: migrate to circle-ci, use vpn for the upload.

I wouldn't mind trying circle-ci out, but VPN for the upload sounds a bit of an overkill...

ghost assigned jpkrohling Sep 14, 2018

ghost added the review label Sep 14, 2018

jpkrohling force-pushed the 536-Move-to-bintray branch from 56aaf17 to acb4295 Compare September 14, 2018 12:07

yurishkuro reviewed Sep 14, 2018

View reviewed changes

jpkrohling force-pushed the 536-Move-to-bintray branch from 7fe4713 to 1912c10 Compare September 17, 2018 16:01

jpkrohling changed the title ~~Add Shipkit for release management~~ [WIP] Add Shipkit for release management Sep 17, 2018

jpkrohling changed the title ~~[WIP] Add Shipkit for release management~~ dd Shipkit for release management Sep 18, 2018

jpkrohling changed the title ~~dd Shipkit for release management~~ Add Shipkit for release management Sep 18, 2018

jpkrohling force-pushed the 536-Move-to-bintray branch from a61b601 to 8dd713b Compare September 18, 2018 07:22

pavolloffay reviewed Sep 18, 2018

View reviewed changes

jpkrohling changed the title ~~Add Shipkit for release management~~ WIP - Add Shipkit for release management Sep 19, 2018

pavolloffay reviewed Sep 19, 2018

View reviewed changes

jpkrohling force-pushed the 536-Move-to-bintray branch 2 times, most recently from 753a7ea to 9762fd5 Compare September 19, 2018 08:36

jpkrohling added 7 commits September 21, 2018 15:09

Add Shipkit for release management

12e5382

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Update version.properties based on the tag

245ed69

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Add 'developers', as required by Maven Central

fd39367

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Update RELEASE.md, as manual version bump isn't required

982f144

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Mention 'snapshot' task for development

c8b5cf0

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Update Gradle to 4.10

bbd3569

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Detail the release process

cdbe0f5

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

jpkrohling force-pushed the 536-Move-to-bintray branch from 78adf15 to cdbe0f5 Compare September 21, 2018 13:10

pavolloffay mentioned this pull request Oct 15, 2018

Migrate from travis to circleci #562

Closed

jpkrohling closed this Jan 21, 2019

ghost removed the review label Jan 21, 2019

		@@ -0,0 +1,2 @@
		version=0.31.1
		previousVersion=0.31.0

WIP - Add Shipkit for release management #549

WIP - Add Shipkit for release management #549

Conversation

jpkrohling commented Sep 14, 2018

Which problem is this PR solving?

Short description of the changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling Sep 18, 2018 • edited Loading

Choose a reason for hiding this comment

pavolloffay Sep 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling Sep 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling Sep 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling commented Sep 17, 2018

Choose a reason for hiding this comment

jpkrohling Sep 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling commented Sep 19, 2018 • edited Loading

codecov bot commented Sep 19, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling commented Sep 19, 2018 • edited Loading

objectiser commented Sep 19, 2018

jpkrohling commented Sep 25, 2018

vprithvi commented Sep 25, 2018

jpkrohling commented Sep 25, 2018

pavolloffay commented Sep 25, 2018 • edited Loading

jpkrohling commented Sep 25, 2018

pavolloffay commented Sep 25, 2018

vprithvi commented Sep 25, 2018

jpkrohling commented Sep 25, 2018

yurishkuro commented Sep 25, 2018

jpkrohling commented Sep 27, 2018 • edited Loading

pavolloffay commented Oct 15, 2018

yurishkuro commented Oct 15, 2018

pavolloffay commented Oct 15, 2018

jpkrohling commented Oct 15, 2018

jpkrohling Sep 18, 2018 •

edited

Loading

pavolloffay Sep 18, 2018 •

edited

Loading

jpkrohling Sep 19, 2018 •

edited

Loading

jpkrohling Sep 14, 2018 •

edited

Loading

jpkrohling Sep 18, 2018 •

edited

Loading

jpkrohling commented Sep 19, 2018 •

edited

Loading

codecov bot commented Sep 19, 2018 •

edited

Loading

jpkrohling commented Sep 19, 2018 •

edited

Loading

pavolloffay commented Sep 25, 2018 •

edited

Loading

jpkrohling commented Sep 27, 2018 •

edited

Loading