<div dir="ltr">If somebody wants to make sure they don't push commits with extra tags, it's possible to set up a local pre-push hook that would complain.<div><br></div><div>In /path/to/llvm-project/.git/hooks/pre-push</div><div>paste the following</div><div><br></div><div>#!/bin/sh<br><br># Disallow pushing commits that contain in their description the Phabricator<br># tags that are unwelcome in the upstream LLVM repository.<br>#<br># This hook is called with the following parameters:<br>#<br># $1 -- Name of the remote to which the push is being done<br># $2 -- URL to which the push is being done<br>#<br># If pushing without using a named remote those arguments will be equal.<br>#<br># Information about the commits which are being pushed is supplied as lines to<br># the standard input in the form:<br>#<br># <local ref> <local sha1> <remote ref> <remote sha1><br>#<br><br># This SHA will be used if the commit is deleted on either side.<br>z40=0000000000000000000000000000000000000000<br><br>while read local_ref local_sha remote_ref remote_sha; do<br> if [ "$local_sha" != "$z40" ]; then<br> if [ "$remote_sha" = $z40 ]; then<br> # New branch, examine all commits<br> range="$local_sha"<br> else<br> # Update to existing branch, examine new commits<br> range="$remote_sha..$local_sha"<br> fi<br><br> # Check for Phabricator tags and stop if found.<br> commit=`git rev-list -n 1 \<br> --grep '^Subscribers:' \<br> --grep '^Tags:' \<br> --grep '^Reviewers:' \<br> "$range"`<br> if [ -n "$commit" ]; then<br> echo >&2 "Found Phabricator tags in $commit, push aborted."<br> echo >&2 "Please remove the lines starting with Subscribers, Tags,"<br> echo >&2 "Reviewers from the commit messages before pushing."<br> exit 1<br> fi<br> fi<br>done<br><br>exit 0<br></div><div><br></div><div>and don't forget to chmod +x on the file.</div><div><br></div><div>This will report the hash of the commit about to be pushed if it contains "Subscribers", "Tags" or "Reviewers" and abort the push.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 6, 2020 at 4:18 PM Joseph Tremoulet via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div class="gmail-m_3523499180101841090WordSection1">
<p class="MsoNormal">My own commits come through that way, with everyone I nominated listed in “reviewers” and then just those that replied in “reviewed by”. I generate the messages using “arc amend”.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>From:</b> James Henderson <<a href="mailto:jh7370.2008@my.bristol.ac.uk" target="_blank">jh7370.2008@my.bristol.ac.uk</a>> <br>
<b>Sent:</b> Monday, January 6, 2020 4:52 AM<br>
<b>To:</b> David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
<b>Cc:</b> Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>>; Joseph Tremoulet <<a href="mailto:jotrem@microsoft.com" target="_blank">jotrem@microsoft.com</a>>; llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> Re: [llvm-dev] [EXTERNAL] Re: Delete Phabricator metadata tags before committing<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">I'm sure I've seen many commits with both "Reviewed by:" and "Reviewers:" tags, which look to have been done with arc (though I can't be sure). How were those generated?<u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Sat, 4 Jan 2020 at 19:12, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<p class="MsoNormal">Yeah, I tend to prune it down myself - and if the list has only one name on it, it's usually a pretty good indication they actually reviewed it. If it has many names, good chance it was just everyone someone put on the list and then I go
check it manually.<u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Sat, Jan 4, 2020 at 7:45 AM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal">The limitation with the "Reviewed by" line is that if you just use `arc` it indicates who was added as reviewer on the revision, but not who approved it. Because of this, I am wary of relying on this line for anything.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">If you want to know who reviewed a change, better click on the Differential Revision link and go to the source of truth.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Mehdi<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Thu, Jan 2, 2020 at 10:44 AM Joseph Tremoulet via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal">+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.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">-Joseph<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>James Henderson via llvm-dev<br>
<b>Sent:</b> Thursday, January 2, 2020 9:57 AM<br>
<b>To:</b> David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
<b>Cc:</b> llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> [EXTERNAL] Re: [llvm-dev] Delete Phabricator metadata tags before committing<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Fri, 27 Dec 2019 at 20:55, David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<p class="MsoNormal">I don't think they're sufficiently problematic to worry about adding more steps people need to be aware of/follow to submit.<br>
<br>
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).<u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Fri, Dec 27, 2019 at 12:48 PM Fangrui Song via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<p class="MsoNormal">Many git commits in the monorepo look like the following:<br>
<br>
[Tag0][Tag1] Title line<br>
<br>
Summary:<br>
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.<br>
<br>
Reviewers: username0, username1<br>
<br>
Reviewed By: username0<br>
<br>
Subscribers: username2, username3, llvm-commits<br>
<br>
Tags: #llvm<br>
<br>
Differential Revision: <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FDxxxxx&data=02%7C01%7Cjotrem%40microsoft.com%7C5cbabf2f837b4d6ca1cb08d7928e1285%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637139011210477226&sdata=ytxz8oxg8MTVu5BhcMLtuAlXq71xVH%2BbDiefGurG70c%3D&reserved=0" target="_blank">
https://reviews.llvm.org/Dxxxxx</a><br>
<br>
These Phabricator metadata lines (`Reviewers:`, `Reviewed By:`, etc) are created automatically when the author uploads the patch via `arc diff 'HEAD^'`.<br>
The summary and metadata can be copied from Phabricator to the local commit via `arc amend`.<br>
<br>
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.<br>
<br>
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
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FDeveloperPolicy.html&data=02%7C01%7Cjotrem%40microsoft.com%7C5cbabf2f837b4d6ca1cb08d7928e1285%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637139011210487186&sdata=imFMd8CPhPcKasrQvedqtvPa7NI2kn5CjkZ8ilg8kEU%3D&reserved=0" target="_blank">
https://llvm.org/docs/DeveloperPolicy.html</a> 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.<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev&data=02%7C01%7Cjotrem%40microsoft.com%7C5cbabf2f837b4d6ca1cb08d7928e1285%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637139011210497143&sdata=DXTMUQPlQsIomb6huyOgqhaktFqvdAS3AmckRaF7j7o%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev&data=02%7C01%7Cjotrem%40microsoft.com%7C5cbabf2f837b4d6ca1cb08d7928e1285%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637139011210497143&sdata=DXTMUQPlQsIomb6huyOgqhaktFqvdAS3AmckRaF7j7o%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev&data=02%7C01%7Cjotrem%40microsoft.com%7C5cbabf2f837b4d6ca1cb08d7928e1285%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637139011210497143&sdata=DXTMUQPlQsIomb6huyOgqhaktFqvdAS3AmckRaF7j7o%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div>Alex</div></div></div></div>