[land-bot] Call land-patch when generating automated commits
ClosedPublic
Actions

Authored by jasonbcox on Sep 17 2020, 15:59.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABCda60e76e598d: [land-bot] Call land-patch when generating automated commits

Summary

This decouples generation of automated commits from the landing logic.
land-patch.sh can be used to land the change generated from this script, so there
is no need to duplicate this logic here.

Depends on D7467

Test Plan

COMMIT_TYPE=archive-release-notes ./automated-commits.sh -p HEAD

Diff Detail

Repository

rABC Bitcoin ABC

Branch

ac-use-lb

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 12773
Build 25619: Build Diff	build-diff · build-without-wallet · build-clang-tidy · build-clang-10
Build 25618: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Sep 17 2020, 15:59

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 17 2020, 15:59

jasonbcox requested review of this revision.Sep 17 2020, 15:59

Harbormaster completed remote builds in B12773: Diff 23554.Sep 17 2020, 16:18

This is doing exactly the wrong things. Instead of consolidating the framework in which automated commit happen, this instead consolidate the fact that there some kind of big ball of mud type script that can do every signle automated commit, but only half the workflow.

contrib/source-control-tools/automated-commits.sh
250	You should be running the script that run the tests and land

This revision now requires changes to proceed.Sep 17 2020, 16:24

Remove all the refactoring and just call the land patch script

jasonbcox retitled this revision from [land-bot] Remove all landing logic from automated-commits script to [land-bot] Call land-patch when generating automated commits.Sep 17 2020, 17:02

jasonbcox edited the summary of this revision. (Show Details)

jasonbcox edited the test plan for this revision. (Show Details)

I' not sure why you would revert back other changes, there were not bad per se.

contrib/source-control-tools/automated-commits.sh
75 ↗	(On Diff #23561)	That's a good way to defeat ccache.

This revision is now accepted and ready to land.Sep 17 2020, 17:10

Harbormaster completed remote builds in B12777: Diff 23561.Sep 17 2020, 17:26

In D7476#177734, @deadalnix wrote:

I' not sure why you would revert back other changes, there were not bad per se.

Honestly some of them belong in their own patches. I shouldn't have done it that way to begin with because this patch as it is now is much easier to review.

contrib/source-control-tools/automated-commits.sh
75 ↗	(On Diff #23561)	We do something similar for all of the CI builds as well. My understanding is the caching performs better if you know the cmake configuration is unlikely to change, and you can make this assumption by the build name. @Fabien can elaborate more on the details. It's possible there's a smarter way to do this by running like-configuration builds in the same directory or something.