[llvm-dev] Delete Phabricator metadata tags before committing

Thu Apr 9 10:14:16 PDT 2020

On 4/9/20 11:47 AM, Hubert Tong via llvm-dev wrote:
 > On Thu, Apr 9, 2020 at 12:29 PM Michael Kruse via llvm-dev <
 > llvm-dev at lists.llvm.org> wrote:
 >
 >> I was always assuming that the suggested commit is assembled in the
 >> PHP code run by arcanist command run locally. If indeed the arc
 >> command requests the commit message from the server, we could do some
 >> additional things:
 >>
 >>  * Remove "Summary:" in front of message
 >>  * Line break after 72 columns
 >>  * Convert summary markdown formatting to plain texts (e.g. remove
 >> backticks; I don't know any git client that renders as markdown)
 >>
 > -1 on removing backticks. The `lowercase` in the middle of a sentence can
 > be confusing without the backticks.

TBH, I don't need markdown to be rendered to make sense to me. If I see
backticks I know that code or commands are meant here, which can
disambiguate common words from variables nicely. I also don't see how
`backticks`, *asterisks*, or __underlines__ hurt the text flow. Maybe it
is just me, since I'm used to read/wrie markdown in vim, but I find they
highlight words just as well in plain text.

I do think we could automate the process in general though. I regularly
forget to run my `arcfilter` command (adopted from Fangrui Song I think):

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

and for now it also breaks single line commit messages (not subjects).

Cheers,
   Johannes

 >>  * Add/check existence of [component] tag
 >>
 >> Alternatively, we could make an upstream feature request
 >>
 >> Michael
 >>
 >> Am Do., 9. Apr. 2020 um 11:16 Uhr schrieb Jon Roelofs via llvm-dev
 >> <llvm-dev at lists.llvm.org>:
 >>>
 >>> Can we fix this in reviews.llvm.org's fork of phab?
 >>>
 >>>
 >> 
https://github.com/phacility/phabricator/blob/cac3dc4983c3671ba4ec841aac8efac10744a80c/src/applications/differential/conduit/DifferentialGetCommitMessageConduitAPIMethod.php
 >>>
 >>> Seems straightforward(-ish) to drop the relevant fields there, that way
 >> `arc land` automatically DTRT.
 >>>
 >>> Jon
 >>>
 >>> On Fri, Dec 27, 2019 at 11:30 PM Mehdi AMINI via llvm-dev <
 >> llvm-dev at lists.llvm.org> wrote:
 >>>>
 >>>>
 >>>>
 >>>> 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?
 >>>>
 >>>>
 >>>> That's be great!
 >>>> I suspect we can provide a bash function to add to one's .bashrc to
 >> make it trivial:
 >>>>
 >>>> function arcfilter() {  git log -1 --pretty=%B | awk '/Reviewers:
 >> /{p=1; sub(/Reviewers: .*Differential Revision: /, "")}; /Differential
 >> Revision: /{p=0;}; !p'   | git commit --amend -F - ; }
 >>>>
 >>>> Just running `arcfilter` before pushing will filter the commit it out
 >> automatically!
 >>>>
 >>>>>
 >>>>> It'd be nice if some pre-receive hooks can be set up to enforce
 >> deleting some really unnecessary metadata tags.
 >>>>
 >>>>
 >>>> Unfortunately you can't add pre-receive hook on GitHub as far as I 
know.
 >>>>
 >>>> --
 >>>> Mehd
 >>>>
 >>>> _______________________________________________
 >>>> LLVM Developers mailing list
 >>>> llvm-dev at lists.llvm.org
 >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
 >>>
 >>> _______________________________________________
 >>> LLVM Developers mailing list
 >>> llvm-dev at lists.llvm.org
 >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
 >> _______________________________________________
 >> LLVM Developers mailing list
 >> llvm-dev at lists.llvm.org
 >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
 >>
 >
 >
 > _______________________________________________
 > LLVM Developers mailing list
 > llvm-dev at lists.llvm.org
 > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev