[docs] avoid 'arc land' command

[trofi]

Summary

Avoid using arc land and suggest cleaning up commit
message from most ofr Phabricator tags like Summary,
Reviewers, Subscribers and Tags.

Suggest keeping Differential Revision and use
git commit --amend for that.

[trofi]

Minor: tweak summary to contain '[docs]' instead of 'docs:'.

[sylvestre.ledru]

are you sure? D<revision> worked for me in the past

[mehdi_amini]

llvm/docs/Phabricator.rst

232–233

I didn't know we were documenting arc land, I think we should discourage it heavily instead.

This is creating commit message with a lot of cruft inside: all the Phabricator tags should be removed before pushing IMO.

[trofi]

In D78867#2007155, @sylvestre.ledru wrote:

are you sure? D<revision> worked for me in the past

I copied arc output right from the terminal. Maybe I'm missing some configuration or arc behaviour changed.

llvm/docs/Phabricator.rst

232–233

Oh, TIL.

I can reword the section to avoid arc land. Would you be fine to review it or maybe you can suggest someone else as well?

[trofi]

Instead of tweaking arc land command add a note to strip all
Phabricator tags except Differential Revision.

[trofi]

llvm/docs/Phabricator.rst

232–233

Reworded to discourage arc land.

[mehdi_amini]

llvm/docs/Phabricator.rst

236

What about adding the same helper function as we document here: https://mlir.llvm.org/getting_started/Contributing/#using-arcanist ?

[sylvestre.ledru]

works for me
I like "arc land" but I am not opposed to changing the doc

[trofi]

Initially I intended to tweak the command to something that just works. I don't think I'm the right person to decide on how the changes should be landed. I'll abandon the change.

[mehdi_amini]

I'll pick this one up then!

[hubert.reinterpretcast]

llvm/docs/Phabricator.rst

206

I am against this. I appreciate being able to locate reviewers associated with an area from the commit message.

[mehdi_amini]

llvm/docs/Phabricator.rst

206

I think the extra tag is "Reviewed By:"?

[lattner]

I'd recommend running this by @klimek

[hubert.reinterpretcast]

llvm/docs/Phabricator.rst

206

Yes; thanks. Also, I find the Summary line useful for separating the commit message body from the subject line on the buildbot.

[mehdi_amini]

llvm/docs/Phabricator.rst

206

That seems something to fix on the buildbot side. Git commit message have well known convention that define the title and the description.

[hubert.reinterpretcast]

llvm/docs/Phabricator.rst

206

I'm fine with that view. Just noting that newer buildbot instances I am aware of still generate the "Comment" with the subject line immediately before the first line of the body.

[klimek]

General note on that patch:
I am fine with both views, I know people who like to use arc land, I know people (like myself), who like to not have noisy messages.
I don't know that we have a good process to really decide that (Chris as BDFL dropped himself out of this one :)
I think giving folks a arcfilter line like Mehdi did is a great idea if we decide to go with the policy.

llvm/docs/Phabricator.rst

::
   arc patch D<Revision>
 
 
 This will create a new branch called ``arcpatch-D<Revision>`` based on the
 current ``master`` and will create a commit corresponding to ``D<Revision>`` with a
 commit message derived from information in the Phabricator review.
 
 Check you are happy with the commit message and amend it if necessary.
-For example, ensure the 'Author' property of the commit is set to the original author.
+For example, ensure that:
+
+* the ``Author`` property of the commit is set to the original author.
+* all phabricator tags except ``Differential Revision`` are stripped
+  from commit message.
+
 You can use a command to correct the author property if it is incorrect:
 
 ::
 
   git commit --amend --author="John Doe <[e-mail]>"
 
-Then, make sure the commit is up-to-date, and commit it. This can be done by running
+
+Otherwise you can clean commit message as:
+
+::
+
+  git commit --amend
+
+
+Then, make sure the commit is up-to-date, and push it. This can be done by running
 the following:
 
 ::
 
   git pull --rebase origin master
   git show # Ensure the patch looks correct.
   ninja check-$whatever # Rerun the appropriate tests if needed.
   git push
 
-Or
-
-::
-
-  arc land D<Revision>
 
+Avoid using `arc land` as it keeps redundant tags in the commit message.
+Redundant tag examples are ``Summary``, ``Reviewers``, ``Subscribers``,
+``Tags``.
 
 Abandoning a change
 -------------------
 
 If you decide you should not commit the patch, you should explicitly abandon
 the review so that reviewers don't think it is still open. In the web UI,
 scroll to the bottom of the page where normally you would enter an overall
 comment. In the drop-down Action list, which defaults to "Comment," you should