[land-bot] Pass committer name/email to land bot endpoint
ClosedPublic
Actions

Authored by jasonbcox on Jul 1 2020, 18:10.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC19491d131486: [land-bot] Pass committer name/email to land bot endpoint

Summary

The land endpoint requires these fields now.

Test Plan

Used to land D6796:
arc land-via-bot --revision D6796

Diff Detail

Repository

rABC Bitcoin ABC

Branch

land-via-bot-committer-info

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11809
Build 23264: Default Diff Build & Tests	Link to the build
Build 23263: Build without Wallet	Link to the build
Build 23262: Build with clang-10	Link to the build
Build 23261: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jul 1 2020, 18:10

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 1 2020, 18:10

jasonbcox requested review of this revision.Jul 1 2020, 18:10

Harbormaster completed remote builds in B11809: Diff 21936.Jul 1 2020, 18:31

jasonbcox mentioned this in D6692: [land-bot] Preserve committer name and email.Jul 2 2020, 22:40

Fabien requested changes to this revision.Jul 3 2020, 07:19

Fabien added a subscriber: Fabien.

Fabien added inline comments.

arcanist/workflow/ArcanistLandBotWorkflow.php
151	You can use arcanist `getAuthor()` method: https://secure.phabricator.com/diffusion/ARC/browse/master/src/repository/api/ArcanistGitAPI.php$798 or even better `getLocalCommitInformation()`: https://secure.phabricator.com/diffusion/ARC/browse/master/src/repository/api/ArcanistGitAPI.php$99 which gives you all the info and provides error management. The later will also prevent the rare edge case where the user changed the config after he committed.

This revision now requires changes to proceed.Jul 3 2020, 07:19

jasonbcox requested review of this revision.Jul 8 2020, 00:34

jasonbcox added inline comments.

arcanist/workflow/ArcanistLandBotWorkflow.php
151	The author is never the committer in the case of backports. Unfortunately there does not appear to be an API for getting committer info via Arcanist.

Fabien requested changes to this revision.Jul 8 2020, 09:05

Fabien added inline comments.

arcanist/workflow/ArcanistLandBotWorkflow.php
151	Weird, I tried `git var GIT_AUTHOR_IDENT` and it always return my name (independendly of the commit). Anyway, it might be intersting to evaluate if the reason why they switched to use this variable (see https://secure.phabricator.com/rARC6cb8d483b23c132da4c82934438cd1af0e5ba32b) is applicable for us. I git config is the solution, then there is still an API for this: https://secure.phabricator.com/diffusion/ARC/browse/master/src/repository/api/ArcanistGitAPI.php$790

This revision now requires changes to proceed.Jul 8 2020, 09:05

Use Arcanist's repository API as much as possible even though there is no direct
call for committer info.

Harbormaster completed remote builds in B11907: Diff 22087.Jul 8 2020, 17:42

Fabien requested changes to this revision.Jul 10 2020, 07:39

Fabien added inline comments.

arcanist/workflow/ArcanistLandBotWorkflow.php
156 ↗	(On Diff #22087)	There is a `GIT_COMMITER_IDENT` which better fit your intent here according to the documentation. And it might be different from `GIT_AUTHOR_IDENT` according to https://git-scm.com/docs/git-commit#_commit_information

This revision now requires changes to proceed.Jul 10 2020, 07:39

Use GIT_COMMITTER_IDENT directly for both name and email. This no longer
uses any part of the Arcanist API, but is heavily influenced from their code.

Harbormaster completed remote builds in B11957: Diff 22178.Jul 10 2020, 18:06

Fabien accepted this revision.Jul 13 2020, 18:30

This revision is now accepted and ready to land.Jul 13 2020, 18:30

Closed by commit rABC19491d131486: [land-bot] Pass committer name/email to land bot endpoint (authored by jasonbcox). · Explain WhyJul 14 2020, 00:01

This revision was automatically updated to reflect the committed changes.

jasonbcox added a commit: rABC19491d131486: [land-bot] Pass committer name/email to land bot endpoint.