Add a check for unstaged changes when generating automated commits
ClosedPublic
Actions

Authored by jasonbcox on Sep 17 2020, 18:50.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABC8f754a4d5734: Add a check for unstaged changes when generating automated commits

Summary

While this check is not particularly helpful when the automation runs
in production, this will help prevent blowing away changes by mistake when
testing this script locally.

It also turns out that the forced git checkout on master isn't really necessary anymore.
While land-patch.sh still assumes working on master, better separation of responsibilities
makes this script easier to test locally without specifying a given parent commit.

Test Plan

COMMIT_TYPE=dummy ./automated-commits.sh ./patch-recipes/update-aur-version.sh bitcoin-abc   # Still bails early since there was no change

echo "" >> CMakeLists.txt
COMMIT_TYPE=archive-release-notes ./automated-commits.sh  # should error instead of blowing away the change

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.Sep 17 2020, 18:50

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 17 2020, 18:50

jasonbcox requested review of this revision.Sep 17 2020, 18:50

Harbormaster completed remote builds in B12781: Diff 23567.Sep 17 2020, 19:13

deadalnix requested changes to this revision.Sep 18 2020, 00:47

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

contrib/source-control-tools/automated-commits.sh
87 ↗	(On Diff #23567)	Then why the git checkout dance? It seems superflous now.

This revision now requires changes to proceed.Sep 18 2020, 00:47

jasonbcox requested review of this revision.Sep 18 2020, 22:36

jasonbcox added inline comments.

contrib/source-control-tools/automated-commits.sh
87 ↗	(On Diff #23567)	It's superfluous if PARENT_COMMIT is the default. But it makes sense to keep this when testing on a stack of diffs on your local machine.

deadalnix added inline comments.Sep 18 2020, 23:11

contrib/source-control-tools/automated-commits.sh
87 ↗	(On Diff #23567)	What problem does this solve?

jasonbcox planned changes to this revision.Sep 21 2020, 17:53

jasonbcox added inline comments.

contrib/source-control-tools/automated-commits.sh
87 ↗	(On Diff #23567)	While writing my response as to why it was necessary, I realized the assumptions within this script have changed. While `land-patch.sh` enforces master as the only branch it will work on, testing this script locally doesn't care since the change will not be landed anyway. It looks like I can cleanup the PARENT_COMMIT entirely.