Page MenuHomePhabricator

Add --commit to automated-commits to make local testing easier
ClosedPublic

Authored by jasonbcox on Feb 12 2020, 00:34.

Details

Summary

Local testing of automated-commits can be made difficult because it currently
assumes building on top of origin/master. The best example of this is working on a stack
of changes locally, where the top of the stack is a change to automated-commits itself
that depends on commits not yet landed on master.

This patch fixes that by enabling the selection of a commit to build on top of other than
latest master.

Test Plan
arc patch D5270

git checkout master
git checkout -b testbranch
git cherry-pick arcpatch-D5270  # only take the changes from the D5270 patch, not its parent (this patch)
COMMIT_TYPE=update-chainparams ./automated-commits.sh  # fails

git checkout arcpatch-D5270
./automated-commits.sh -h
COMMIT_TYPE=update-chainparams ./automated-commits.sh --parent HEAD  # succeeds
DRY_RUN=no COMMIT_TYPE=update-chainparams ./automated-commits.sh --parent HEAD  # fails

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested changes to this revision.Feb 13 2020, 09:54
Fabien added a subscriber: Fabien.

You should probably forbid to push (no dry run) if commit is not origin/master.

contrib/source-control-tools/automated-commits.sh
42 ↗(On Diff #16309)

shift after exit ?

48 ↗(On Diff #16309)

You should exit here.

This revision now requires changes to proceed.Feb 13 2020, 09:54
  • Fixes according to feedback
  • Small refactor so error messages will appear early
  • Rename --commit to --parent since this is more self explanatory. Associated variable names changed as well.
contrib/source-control-tools/automated-commits.sh
42 ↗(On Diff #16309)

copy paste gone bad from an already bad script. see D5285

Fabien requested changes to this revision.Feb 13 2020, 21:20
Fabien added inline comments.
contrib/source-control-tools/automated-commits.sh
61 ↗(On Diff #16367)

DRY_RUN=yes => DRY_RUN=no

This revision now requires changes to proceed.Feb 13 2020, 21:20

Fixed confusing double-negative

This revision is now accepted and ready to land.Feb 14 2020, 09:18