Integrate with MiMa - Githubissues

DanSimon commented 7 years ago

MiMa is a SBT plugin that can help us detect binary incompatibility across builds. This will be extremely helpful in ensuring that 0.n.x builds are binary compatible for all x, something we've been attempting to enforce but only as a guideline.

aliyakamercan commented 7 years ago

I think we should start using sbt-release and add MiMa as a build step.

DanSimon commented 7 years ago

Can you elaborate on using sbt-release? What part of the existing build process would it replace/improve?

aliyakamercan commented 7 years ago

I was thinking of making mimaReportBinaryIssues part of release process. However, I am not sure if that would be helpful.

Besides that, I think replacing whole build process with sbt-release would be nice. Currently, everything is manual from tagging to publishing to maven central.

DanSimon commented 7 years ago

Sounds like a pretty reasonable idea, as long as it can actually make things cleaner/more concise. Two things though:

Let's make that a separate issue from MiMa integration
We also need to look at getting rid of Build.scala in general, since it's deprecated. So maybe all of this can be wrapped into one larger "cleanup build process" task.

benblack86 commented 7 years ago

https://github.com/tumblr/colossus/issues/546 https://github.com/tumblr/colossus/issues/541

nsauro commented 7 years ago

Howdy ya'll.

Need some help with this? I've been doing a lot of SBT stuff lately, and can probably pitch in here.

nsauro commented 7 years ago

In general, I've taken a pretty long hiatus from this project, mainly b/c I was super busy with other stuff, but I should start having more free time again, and would like to pitch in where i can.

benblack86 commented 7 years ago

Who hacked Nick's account?

benblack86 commented 7 years ago

Welcome back @nsauro. Do you want to look into this? Not sure we want to have it automatically error a build or just provide the information.

benblack86 commented 7 years ago

If you contribute can you also sign the CLA :)

https://github.com/tumblr/colossus/blob/master/CONTRIBUTING.md

nsauro commented 7 years ago

I can sign the CLA, though I might already have?

So, which is more important? Mima or sbt-release? If mima is a thing you want to use, do you want to use it during the release phase, or during compilation/tests?

The one pain about Mima is tracking the previous versions you want to run mima against. It would probably be less than ideal to go through sbt-release integration to avoid having to manually modify versions/tags/etc, only to then have to manually track previous versions. What level of automation do you want around this? We could probably do something around having mima read in previous versions from a file which is appended to at release time. At least then we are automatically generating and using a list of released versions. Removing from that list would be manual however.

aliyakamercan commented 7 years ago

Hey @nsauro,

I recently merged #562, I think we will not use sbt-release for now. Maybe we can use sbt-git to get the latest version from the latest tag?

nsauro commented 7 years ago

Out of curiosity, why not use sbt-release? It really does trivialize the release process and is highly customizable. For colossus, the only customizations you would need would be to replace publish, with publish-signed(at least at first glance :) ). All of the tagging, git integration, etc is taken care of. I believe you would need to make sure that travis can commit to colossus, so you might need to generate a key for that.

If you choose to use mima, you could then do it as part of the release process. The one thing I'm struggling on, is this too late in the process for mima?

aliyakamercan commented 7 years ago

After the latest changes, we can build, publish and release with pushing a version tag.

sbt-release complicates travis-ci integration more. It will be harder to control versions, we will have to have some extra logic when we want something other than major version bump. We will have to skip commits back to master etc. I don't think we gain much from using it.

I think release/publish step is a good place run mima, PRs can have non-binary compatible changes if they are going in a major release. Mima should only run for minor version bumps, probably before publish.

benblack86 commented 7 years ago

Might want to see what akka does since they seem to use MiMa: https://github.com/akka/akka/blob/master/CONTRIBUTING.md#binary-compatibility

nsauro commented 7 years ago

Just a heads up. I'm out of town until after Labor Day :)

nsauro commented 7 years ago

So, I'm starting to poke around at this, and have it working at least in a very basic fashion.

What is the desired behavior here?

Is this to be run manually during the deploy process?
How do we want to populate the key mimaPreviousArtifacts which controls if the plugin is enabled for a project, and what previous versions to run against? To be controlled manually as well? This would mean that at publish time, we would need to make sure this value is updated in the build.sbt.
What updates (if any) to the travis build do you want? To run this plugin based on a certain set of conditions? Are the conditions that @aliyakamercan mentioned: I think release/publish step is a good place run mima, PRs can have non-binary compatible changes if they are going in a major release. Mima should only run for minor version bumps, probably before publish. The only set of conditions?

benblack86 commented 7 years ago

Based on the new CONTRIBUTING.md, using the GitFlow model will mean we wouldn't publish that many minor builds - only if a hotfix is needed. Meaning this wouldn't actually be a problem. It might be nice to document the interface changes between x.y and x.Y+1 so it is easier for people to upgrade. Maybe we should just add it as a plugin, but leave it as something that is manually run when writing the release notes. Also we should write release notes :)

nsauro commented 7 years ago

Ok, I will add this as a plugin, and should have something to go maybe by this evening.

nsauro commented 7 years ago

So, I was getting ready to push a branch, and was looking at what I should set the value to for mimaPreviousVersions.

I'm a bit confused.

The latest version in develop/master is 0.9.0-SNAPSHOT, yet: a 0.9.1 exists in the wild

https://mvnrepository.com/artifact/com.tumblr/colossus_2.11

So my 2 questions are:

What should I set the previous builds to check mima against?
Is version tracking out of sync? Did we cut a release and forget to bump some versions? Or am I just looking in the wrong place?

Are we following Semver?

nsauro commented 7 years ago

Follow up question: Are we following Semver?

DanSimon commented 7 years ago

Oh yeah, develop should definitely be on 0.10.0-SNAPSHOT, master should be on 0.9.2-SNAPSHOT.

We're not doing semver in the strict sense that any breaking change causes a major version bump, for now we're still in 0.x versions. But we are doing it for minor and hotfix versions. So 0.10.0 and 0.11.0 need not be binary compatible, but 0.10.0 and 0.10.1 should be.

benblack86 commented 7 years ago

I think the problem is that version bumps previously happened in a branch that got reverted and hence didn't update master/develop.

nsauro commented 7 years ago

I still think we should look into sbt-release, as that could potentially alleviate these kinds of issues. However, gitflow would mean we would need a custom process in sbt-release, which isn't in itself, too bad, but does complicate any potential integration. NBD for now.

So, for previous version, should I just use what is released right now?

benblack86 commented 7 years ago

Sure. If we use sbt-release, how will it work with travis?

nsauro commented 7 years ago

I don't think it will, at least not easily. At a side gig I once set up sbt-release with any commit to master that had a special tag in the message would kick off sbt-release. It worked..mostly. The biggest pain being that a commit to master was required..even if there was nothing that changed. The travis.yml file here indicates that there is some level of determining if a release should be kicked off by travis via publishSigned. That could be potentially swapped to use release with-defaults, as a starting point.

I'm not super familiar with how colossus is released, as I've never done it, so I can't say for certain how good a fit this is..but I can say from experience, sbt-release does a great job of allowing for a repeatable, automated way to do release and all the clerical work(version bumps, tags, etc) involved. Idk how involved we'd have to make the custom process to work with gitflow however. Also depends on how much(if any) of the work you'd want to have sbt-release do.

benblack86 commented 7 years ago

Currently process is described here: https://github.com/tumblr/colossus/pull/562, basically a tag push to master.

benblack86 commented 7 years ago

https://github.com/tumblr/colossus/pull/583

tumblr / colossus

Integrate with MiMa #506