[llvm-dev] [EXTERNAL] Re: Delete Phabricator metadata tags before committing

[Fangrui Song via llvm-dev]

On 2020-01-02, Joseph Tremoulet via llvm-dev wrote:
>+1 to keeping “Reviewed by”.  Whenever I’m fixing a bug in unfamiliar code, I
>look at both author and “reviewed by” from the history of that code to help
>pick reviewers for my change.
>
>-Joseph
>
>From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of James Henderson
>via llvm-dev
>Sent: Thursday, January 2, 2020 9:57 AM
>To: David Blaikie <dblaikie at gmail.com>
>Cc: llvm-dev <llvm-dev at lists.llvm.org>
>Subject: [EXTERNAL] Re: [llvm-dev] Delete Phabricator metadata tags before
>committing
>
>I also find the "Reviewed by" tag useful (as well as the review link), for the
>same reasons. In fact, I don't even use arcanist to push commits, so I do it
>all by hand, and only include the "Reviewed by" and "Differential Revision"
>tags.
>
>On Fri, 27 Dec 2019 at 20:55, David Blaikie via llvm-dev <
>llvm-dev at lists.llvm.org> wrote:
>
>    I don't think they're sufficiently problematic to worry about adding more
>    steps people need to be aware of/follow to submit.
>
>    Maybe more advisory "you can remove the unneeded tags" might not hurt (but
>    still adds to the length of new contributor documentation, etc, which isn't
>    free) - though "Reviewed By" is useful (in addition to "Differential
>    Revision") if it accurately reflects who reviewed/signed off on the change
>    (makes it easy when I'm reading the mailing list to see who's already
>    looked a patch over, etc).
>
>     
>
>    On Fri, Dec 27, 2019 at 12:48 PM Fangrui Song via llvm-dev <
>    llvm-dev at lists.llvm.org> wrote:
>
>        Many git commits in the monorepo look like the following:
>
>           [Tag0][Tag1] Title line
>
>           Summary:
>           Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque
>        mauris neque, porta nec tristique at, sagittis vel nisi. Fusce pharetra
>        nunc et mauris consequat venenatis.
>
>           Reviewers: username0, username1
>
>           Reviewed By: username0
>
>           Subscribers: username2, username3, llvm-commits
>
>           Tags: #llvm
>
>           Differential Revision: https://reviews.llvm.org/Dxxxxx
>
>        These Phabricator metadata lines (`Reviewers:`, `Reviewed By:`, etc)
>        are created automatically when the author uploads the patch via `arc
>        diff 'HEAD^'`.
>        The summary and metadata can be copied from Phabricator to the local
>        commit via `arc amend`.
>
>        Among the metadata tags, `Differential Revision:` is the most useful
>        one. It associates a commit with a Phabricator differential (Dxxxxx)
>        and allows the differential to be closed automatically when the commit
>        is pushed to llvm-project master.
>
>        The other tags (Subscribers:, Reviewers:, Reviewed By:, Tags:, and the
>        `Summary:` line) are not useful. They just clutter up the commit
>        message. Shall we mention on https://llvm.org/docs/DeveloperPolicy.html
>        that developers are recommended to delete metadata tags before
>        committing? It'd be nice if some pre-receive hooks can be set up to
>        enforce deleting some really unnecessary metadata tags.

I kept `Reviewed By:` before and hestitated whether I should continue
this practice. Nice to see more rationale:)

I am now using a variant of Mehdi's shell function to keep Reviewed By: and Differential Revision:

function arcfilter() { git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:/' | git commit --amend -F -; }