Fangrui Song via llvm-dev
2019-Dec-27 20:48 UTC
[llvm-dev] Delete Phabricator metadata tags before committing
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.
David Blaikie via llvm-dev
2019-Dec-27 20:54 UTC
[llvm-dev] Delete Phabricator metadata tags before committing
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. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191227/8bbff6e2/attachment.html>
Mehdi AMINI via llvm-dev
2019-Dec-28 06:29 UTC
[llvm-dev] Delete Phabricator metadata tags before committing
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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191227/72d31a0c/attachment.html>
David Blaikie via llvm-dev
2019-Dec-28 16:51 UTC
[llvm-dev] Delete Phabricator metadata tags before committing
Maybe there's a way to make an on-save filter that correctly detects that the file being saved is a commit message (bonus if it can detect that it's a commit message for the llvm repo)? On Fri, Dec 27, 2019 at 10: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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191228/e00da4ae/attachment.html>
James Henderson via llvm-dev
2020-Jan-02 14:57 UTC
[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. >> _______________________________________________ >> 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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200102/17d79e1d/attachment.html>
Jon Roelofs via llvm-dev
2020-Apr-09 16:15 UTC
[llvm-dev] Delete Phabricator metadata tags before committing
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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200409/f19dafb6/attachment.html>
Maybe Matching Threads
- Delete Phabricator metadata tags before committing
- Delete Phabricator metadata tags before committing
- [EXTERNAL] Re: Delete Phabricator metadata tags before committing
- [EXTERNAL] Re: Delete Phabricator metadata tags before committing
- Splitting a DF into rows according to a column