From 5713faa66776d551a3ea96944afad8e8a2e3755d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mis=CC=8Cko=20Hevery?= Date: Thu, 25 Jan 2018 15:55:00 -0800 Subject: [PATCH] build: autosquashes SHAs as part of merge-pr script (#21791) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To support `git checkin --fixup` and `git checkin —squash` we need to make sure that `merge-pr` squashes the sepecial commits before they are merged. For more details see: https://robots.thoughtbot.com/autosquashing-git-commits PR Close #21791 --- scripts/github/merge-pr | 81 +++++++++++-------- .../validate-commit-message.js | 15 ++-- .../validate-commit-message.spec.js | 30 ++++--- 3 files changed, 76 insertions(+), 50 deletions(-) diff --git a/scripts/github/merge-pr b/scripts/github/merge-pr index c240403d6d..772967a5b5 100755 --- a/scripts/github/merge-pr +++ b/scripts/github/merge-pr @@ -5,15 +5,32 @@ set -u -e -o pipefail BASEDIR=$(dirname "$0") BASEDIR=`(cd $BASEDIR; pwd)` -if [ $# -eq 0 ]; then +PR_NUMBER=0 +PUSH_UPSTREAM=1 +while [[ $# -gt 0 ]] +do + key="$1" + + case $key in + --dryrun) + PUSH_UPSTREAM=0 + shift # past argument + ;; + *) # unknown option + PR_NUMBER="$1" # save it in an array for later + shift # past argument + ;; + esac +done + +if [ "$PR_NUMBER" -eq 0 ]; then echo "Merge github PR into the target branches" echo - echo "$0 PR_NUMBER" + echo "$0 PR_NUMBER [--dryrun]" echo exit 0 fi -PR_NUMBER="$1" CURRENT_BRANCH=`git rev-parse --abbrev-ref HEAD` PR_SHA_COUNT=`curl -s https://api.github.com/repos/angular/angular/pulls/$PR_NUMBER | node $BASEDIR/utils/json_extract.js commits` PR_LABELS=`curl -s https://api.github.com/repos/angular/angular/issues/$PR_NUMBER/labels` @@ -51,63 +68,59 @@ fi CHECKOUT_MASTER="git checkout merge_pr_master" -CHECKOUT_PATCH="git checkout merge_pr_$PATCH_BRANCH" +CHECKOUT_PATCH="git checkout merge_pr_patch" RESTORE_BRANCH="git checkout $CURRENT_BRANCH" -FETCH_PR="git fetch git@github.com:angular/angular.git pull/$PR_NUMBER/head:angular/pr/$PR_NUMBER heads/master:merge_pr_master heads/$PATCH_BRANCH:merge_pr_$PATCH_BRANCH -f" -PUSH_BRANCHES="git push git@github.com:angular/angular.git merge_pr_master:master merge_pr_$PATCH_BRANCH:$PATCH_BRANCH" -CHERRY_PICK_PR="git cherry-pick angular/pr/$PR_NUMBER~$PR_SHA_COUNT..angular/pr/$PR_NUMBER" -REWRITE_MESSAGE="git filter-branch -f --msg-filter \"$BASEDIR/utils/github_closes.js $PR_NUMBER\" HEAD~$PR_SHA_COUNT..HEAD" +FETCH_PR="git fetch git@github.com:angular/angular.git pull/$PR_NUMBER/head:merge_pr heads/master:merge_pr_master heads/$PATCH_BRANCH:merge_pr_patch -f" +BASE_PR="git checkout merge_pr~$PR_SHA_COUNT -B merge_pr_base" +PUSH_BRANCHES="git push git@github.com:angular/angular.git merge_pr_master:master merge_pr_patch:$PATCH_BRANCH" +SQUASH_PR="git rebase --autosquash --interactive merge_pr_base merge_pr" +CHERRY_PICK_PR="git cherry-pick merge_pr_base..merge_pr" +REWRITE_MESSAGE="git filter-branch -f --msg-filter \"$BASEDIR/utils/github_closes.js $PR_NUMBER\" merge_pr_base..merge_pr" echo "======================" echo "GitHub Merge PR Steps" echo "======================" echo " $FETCH_PR" +echo " $BASE_PR" +echo " $SQUASH_PR" +echo " $REWRITE_MESSAGE" if [[ $MERGE_MASTER == 1 ]]; then - echo " $CHECKOUT_MASTER" - echo " $CHERRY_PICK_PR" - echo " $REWRITE_MESSAGE" + echo " $CHECKOUT_MASTER && $CHERRY_PICK_PR" fi if [[ $MERGE_PATCH == 1 ]]; then - echo " $CHECKOUT_PATCH" - echo " $CHERRY_PICK_PR" - echo " $REWRITE_MESSAGE" + echo " $CHECKOUT_PATCH && $CHERRY_PICK_PR" fi echo " $PUSH_BRANCHES" echo " $RESTORE_BRANCH" echo "----------------------" - +echo ">>> Fetch PR: $FETCH_PR" $FETCH_PR +echo ">>> Mark base: $BASE_PR" +$BASE_PR +echo ">>> Autosquash: $SQUASH_PR" +GIT_EDITOR=echo $SQUASH_PR +echo ">>> Rewrite message: $REWRITE_MESSAGE" +# Next line should work, but it errors, hence copy paste the command. +# $REWRITE_MESSAGE +git filter-branch -f --msg-filter "$BASEDIR/utils/github_closes.js $PR_NUMBER" merge_pr_base..merge_pr if [[ $MERGE_MASTER == 1 ]]; then echo - echo ">>> Checkout master: $CHECKOUT_MASTER" + echo ">>> Cherry pick to master: $CHECKOUT_MASTER && $CHERRY_PICK_PR" $CHECKOUT_MASTER - echo - echo ">>> Cherry pick pr: $CHERRY_PICK_PR" $CHERRY_PICK_PR - echo - echo ">>> Rewrite message: $REWRITE_MESSAGE" - # Next line should work, but it errors, hence copy paste the command. - # $REWRITE_MESSAGE - git filter-branch -f --msg-filter "$BASEDIR/utils/github_closes.js $PR_NUMBER" HEAD~$PR_SHA_COUNT..HEAD - fi if [[ $MERGE_PATCH == 1 ]]; then echo - echo ">>> Checkout $PATCH_BRANCH: $CHECKOUT_PATCH" + echo ">>> Cherry pick to path: $CHECKOUT_PATCH && $CHERRY_PICK_PR" $CHECKOUT_PATCH - echo - echo ">>> Cherry pick pr: $CHERRY_PICK_PR" $CHERRY_PICK_PR - echo - echo ">>> Rewrite message: $REWRITE_MESSAGE" - # Next line should work, but it errors, hence copy paste the command. - # $REWRITE_MESSAGE - git filter-branch -f --msg-filter "$BASEDIR/utils/github_closes.js $PR_NUMBER" HEAD~$PR_SHA_COUNT..HEAD fi $RESTORE_BRANCH -echo ">>> Push branches to angular repo" -$PUSH_BRANCHES +if [[ $PUSH_UPSTREAM == 1 ]]; then + echo ">>> Push branches to angular repo" + $PUSH_BRANCHES +fi echo echo ">>>>>> SUCCESS <<<<<< PR#$PR_NUMBER merged." diff --git a/tools/validate-commit-message/validate-commit-message.js b/tools/validate-commit-message/validate-commit-message.js index f3bcee9c5c..a413cbc87a 100644 --- a/tools/validate-commit-message/validate-commit-message.js +++ b/tools/validate-commit-message/validate-commit-message.js @@ -20,30 +20,35 @@ const fs = require('fs'); const path = require('path'); const configPath = path.resolve(__dirname, './commit-message.json'); const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); -const PATTERN = /^(revert\: )?(\w+)(?:\(([^)]+)\))?\: (.+)$/; +const PATTERN = /^(\w+)(?:\(([^)]+)\))?\: (.+)$/; +const FIXUP_SQUASH = /^(fixup|squash)\! /i; +const REVERT = /^revert: (\"(.*)\"|(.*))?$/i; module.exports = function(commitSubject) { + commitSubject = commitSubject.replace(FIXUP_SQUASH, ''); + commitSubject = commitSubject.replace(REVERT, function(m, g1, g2, g3) { return g2 || g3; }); + if (commitSubject.length > config['maxLength']) { error(`The commit message is longer than ${config['maxLength']} characters`, commitSubject); return false; } const match = PATTERN.exec(commitSubject); - if (!match || match[2] === 'revert') { + if (!match) { error( - `The commit message does not match the format of "(): OR revert: type(): "`, + `The commit message does not match the format of '(): ' OR 'Revert: "type(): "'`, commitSubject); return false; } - const type = match[2]; + const type = match[1]; if (config['types'].indexOf(type) === -1) { error( `${type} is not an allowed type.\n => TYPES: ${config['types'].join(', ')}`, commitSubject); return false; } - const scope = match[3]; + const scope = match[2]; if (scope && !config['scopes'].includes(scope)) { error( diff --git a/tools/validate-commit-message/validate-commit-message.spec.js b/tools/validate-commit-message/validate-commit-message.spec.js index 6813bfe966..138b2f5d19 100644 --- a/tools/validate-commit-message/validate-commit-message.spec.js +++ b/tools/validate-commit-message/validate-commit-message.spec.js @@ -9,6 +9,7 @@ describe('validate-commit-message.js', function() { var validateMessage = require('./validate-commit-message'); var SCOPES = validateMessage.config.scopes.join(', '); + var TYPES = validateMessage.config.types.join(', '); var errors = []; var logs = []; @@ -40,6 +41,10 @@ describe('validate-commit-message.js', function() { expect(validateMessage('test(packaging): something')).toBe(VALID); expect(validateMessage('release: something')).toBe(VALID); expect(validateMessage('release(packaging): something')).toBe(VALID); + expect(validateMessage('release(packaging): something')).toBe(VALID); + expect(validateMessage('fixup! release(packaging): something')).toBe(VALID); + expect(validateMessage('squash! release(packaging): something')).toBe(VALID); + expect(validateMessage('Revert: "release(packaging): something"')).toBe(VALID); expect(errors).toEqual([]); }); @@ -88,21 +93,24 @@ describe('validate-commit-message.js', function() { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - 'INVALID COMMIT MSG: "not correct format"\n => ERROR: The commit message does not match the format of "(): OR revert: type(): "' + 'INVALID COMMIT MSG: "not correct format"\n => ERROR: The commit message does not match the format of \'(): \' OR \'Revert: "type(): "\'' ]); }); - it('should support "revert: type(scope):" syntax and reject "revert(scope):" syntax', function() { - let correctMsg = 'revert: fix(compiler): reduce generated code payload size by 65%'; - expect(validateMessage(correctMsg)).toBe(VALID); + it('should support "revert: type(scope):" syntax and reject "revert(scope):" syntax', + function() { + let correctMsg = 'revert: fix(compiler): reduce generated code payload size by 65%'; + expect(validateMessage(correctMsg)).toBe(VALID); - let incorretMsg = 'revert(compiler): reduce generated code payload size by 65%'; - expect(validateMessage(incorretMsg)).toBe(INVALID); - expect(errors).toEqual([ - 'INVALID COMMIT MSG: "revert(compiler): reduce generated code payload size by 65%"\n => ERROR: The commit message does not match the format of "(): OR revert: type(): "' - ]); - }); + let incorretMsg = 'revert(compiler): reduce generated code payload size by 65%'; + expect(validateMessage(incorretMsg)).toBe(INVALID); + expect(errors).toEqual([ + 'INVALID COMMIT MSG: "revert(compiler): reduce generated code payload size by 65%"\n' + + ' => ERROR: revert is not an allowed type.\n' + + ' => TYPES: ' + TYPES + ]); + }); it('should validate type', function() { @@ -110,7 +118,7 @@ describe('validate-commit-message.js', function() { expect(errors).toEqual( ['INVALID COMMIT MSG: "weird($filter): something"\n' + ' => ERROR: weird is not an allowed type.\n' + - ' => TYPES: build, ci, docs, feat, fix, perf, refactor, release, style, test']); + ' => TYPES: ' + TYPES]); });