Skip to content
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

New release and pr-commit scripts #860

Merged
merged 2 commits into from
Oct 9, 2015

Conversation

fnothaft
Copy link
Member

@fnothaft fnothaft commented Oct 8, 2015

These two commits contain two changes:

  1. The first commit modifies the release scripts to move to a branch-then-release model.
  2. The second commit adds a script script/commit-pr.sh that automates the process of FF merging a PR from the command line.

Branch-then-release model

Recently, someone (@ryan-williams?) mentioned the idea of moving to a branch-then-release model. Here, we would make a branch, and then make the release off of this branch. The release commits would not show up on the master branch. This would make it easier to make maintenance releases. Although the Maven release plugin contains a goal for releasing from a branch, the general consensus is that this goal works with SVN but not with Git (I tested this out and this does seem to be the case). It seems like many people advocate the "Gitflow" release approach; however, that seemed like a radical departure from our current model, so I decided not to go that route.

In this PR, we make an update on the starting branch (presumably master, but this is not hardcoded in the scripts, in case we want to make a maintenance release) to the CHANGES.md file. We then make a branch for the Scala 2.10 release, and run the typical Maven release process there. Finally, we make a branch from our starting branch for the Scala 2.11 release. On this branch, we move the pom.xml files from Scala 2.10 to Scala 2.11, and then run the Maven release process. If we start with this structure:

screen shot 2015-10-08 at 2 41 20 pm

Once the release completes, our repo will look like this:

screen shot 2015-10-08 at 12 53 21 pm

Once both Scala 2.10 and 2.11 releases succeed, the release script updates the pom.xml's on the master branch to the new development version.

PR Commit scripts

There are two problems with the PR merge button in Github:

  1. The button creates a merge commit per PR.
  2. The hook does not require all merges to be fast-forwards; this means that changes can be merged even if they are not based on the latest commit on master.

The ./scripts/commit-pr.sh script will checkout a single PR, rebase those changes on master, and then merge the changes into master as a fast-forward merge. If there are multiple commits in a pull request, the script will prompt the user as to whether they would like to continue with the merge. If it is a PR where the multiple commits should not be squashed down (like this one), the user can say y to continue, and the merge will go on. If the commits should be squashed, the user can say n to stop the merge, and then manually squash the commits from there. The script will print out the directions for finishing the merge, in the case that it aborts.

If we want to merge a pull request (#1) started from the docs branch in the picture below:

screen shot 2015-10-08 at 12 54 06 pm

We would run ./scripts/commit-pr.sh 1, which will do the above, and lead to the following branch structure:

screen shot 2015-10-08 at 1 43 23 pm

@laserson
Copy link
Contributor

laserson commented Oct 8, 2015

Looks awesome, @fnothaft! One question: with the commit-pr.sh script, will it automatically perform a rebase? It's probably safer that the script should only automatically proceed if the PR branch is directly dnstream from master. If not, the person should manually rebase onto master, no?

@heuermh
Copy link
Member

heuermh commented Oct 8, 2015

One thing I noticed with the 0.17.1 release process, the link to CHANGES.md on the releases page does not contain the 0.17.1-related changes

https://github.com/bigdatagenomics/adam/blob/adam-parent_2.11-0.17.1/CHANGES.md

linked from here

https://github.com/bigdatagenomics/adam/releases/tag/adam-parent_2.11-0.17.1

→ Release Notes

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/992/
Test PASSed.

@fnothaft
Copy link
Member Author

fnothaft commented Oct 8, 2015

@heuermh

One thing I noticed with the 0.17.1 release process, the link to CHANGES.md on the releases page does not contain the 0.17.1-related changes

This PR resolves that issue.

@laserson

One question: with the commit-pr.sh script, will it automatically perform a rebase? It's probably safer that the script should only automatically proceed if the PR branch is directly dnstream from master. If not, the person should manually rebase onto master, no?

That is correct; it will automatically rebase. My general thought is that as long as the branch has no merge conflicts, doing a rebase will achieve the same results as Jenkins achieves when it merges the branch into master when testing in the PR builder.

@fnothaft fnothaft merged commit b3206bd into bigdatagenomics:master Oct 9, 2015
@fnothaft fnothaft deleted the release-commit-scripts branch October 9, 2015 13:28
@laserson
Copy link
Contributor

@fnothaft (catching up on some GH notifications) So Jenkins doesn't just check out the PR branch, but it actually creates a merge commit before running tests? This is not how I would expect it to work. In my experience, there have definitely been rebases/merges that finish smoothly but produce unexpected results.

@fnothaft
Copy link
Member Author

If we look at the latest ADAM-prb build, it is doing:

Started by upstream project "ADAM-prb" build number 1002
originally caused by:
 GitHub pull request #858 of commit e6c33c1f0a03f323c21d3f2ce142ce583bc75557 automatically merged.
[EnvInject] - Loading node environment variables.
Building remotely on amp-jenkins-worker-04 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.0/SCALAVER/2.11/SPARK_VERSION/1.4.1/label/centos
Cloning the remote Git repository
Cloning repository https://github.com/bigdatagenomics/adam.git
 > git init /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.0/SCALAVER/2.11/SPARK_VERSION/1.4.1/label/centos # timeout=10
Fetching upstream changes from https://github.com/bigdatagenomics/adam.git
 > git --version # timeout=10
 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10
Fetching upstream changes from https://github.com/bigdatagenomics/adam.git
 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/*:refs/remotes/origin/pr/*
Checking out Revision fcf1d09d57b84b9222f4de34b8941d27348328c9 (origin/pr/858/merge)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f fcf1d09d57b84b9222f4de34b8941d27348328c9
 > git rev-list fac5d5fa86324aebc4128afcbf4bf70a6d4bf9c7 # timeout=10
First time build. Skipping changelog.
[centos] $ /bin/bash /tmp/hudson2880986315095922623.sh

The last line (/bin/bash /tmp/hudson2880986315095922623.sh) runs the start of the build in Jenkins. The important lines are:

 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/*:refs/remotes/origin/pr/*
Checking out Revision fcf1d09d57b84b9222f4de34b8941d27348328c9 (origin/pr/858/merge)

Long story short, the branch origin/pr/<number>/merge is a branch that corresponds to what you would get if you pressed the Github merge button for PR <number>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants