[llvm-dev] [EXTERNAL] Re: Delete Phabricator metadata tags before committing
Alex Zinenko via llvm-dev
llvm-dev at lists.llvm.org
Wed Jan 15 04:15:15 PST 2020
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.
In /path/to/llvm-project/.git/hooks/pre-push
paste the following
#!/bin/sh
# Disallow pushing commits that contain in their description the Phabricator
# tags that are unwelcome in the upstream LLVM repository.
#
# This hook is called with the following parameters:
#
# $1 -- Name of the remote to which the push is being done
# $2 -- URL to which the push is being done
#
# If pushing without using a named remote those arguments will be equal.
#
# Information about the commits which are being pushed is supplied as lines
to
# the standard input in the form:
#
# <local ref> <local sha1> <remote ref> <remote sha1>
#
# This SHA will be used if the commit is deleted on either side.
z40=0000000000000000000000000000000000000000
while read local_ref local_sha remote_ref remote_sha; do
if [ "$local_sha" != "$z40" ]; then
if [ "$remote_sha" = $z40 ]; then
# New branch, examine all commits
range="$local_sha"
else
# Update to existing branch, examine new commits
range="$remote_sha..$local_sha"
fi
# Check for Phabricator tags and stop if found.
commit=`git rev-list -n 1 \
--grep '^Subscribers:' \
--grep '^Tags:' \
--grep '^Reviewers:' \
"$range"`
if [ -n "$commit" ]; then
echo >&2 "Found Phabricator tags in $commit, push aborted."
echo >&2 "Please remove the lines starting with Subscribers,
Tags,"
echo >&2 "Reviewers from the commit messages before pushing."
exit 1
fi
fi
done
exit 0
and don't forget to chmod +x on the file.
This will report the hash of the commit about to be pushed if it contains
"Subscribers", "Tags" or "Reviewers" and abort the push.
On Mon, Jan 6, 2020 at 4:18 PM Joseph Tremoulet via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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”.
>
>
>
> *From:* James Henderson <jh7370.2008 at my.bristol.ac.uk>
> *Sent:* Monday, January 6, 2020 4:52 AM
> *To:* David Blaikie <dblaikie at gmail.com>
> *Cc:* Mehdi AMINI <joker.eph at gmail.com>; Joseph Tremoulet <
> jotrem at microsoft.com>; llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* Re: [llvm-dev] [EXTERNAL] Re: Delete Phabricator metadata tags
> before committing
>
>
>
> 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?
>
>
>
> On Sat, 4 Jan 2020 at 19:12, David Blaikie <dblaikie at gmail.com> wrote:
>
> 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.
>
>
>
> On Sat, Jan 4, 2020 at 7:45 AM Mehdi AMINI <joker.eph at gmail.com> wrote:
>
> 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.
>
>
>
> If you want to know who reviewed a change, better click on the
> Differential Revision link and go to the source of truth.
>
>
>
> --
>
> Mehdi
>
>
>
> On Thu, Jan 2, 2020 at 10:44 AM Joseph Tremoulet via llvm-dev <
> llvm-dev at lists.llvm.org> 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
> <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>
>
> 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
> <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>
> 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.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> <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>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> <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>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> <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>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
--
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200115/99c95c87/attachment.html>
More information about the llvm-dev
mailing list