[PATCH] D61267: Update Phabricator.rst

Wink Saville via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 11:57:01 PDT 2019


winksaville added inline comments.


================
Comment at: llvm/docs/Phabricator.rst:69-70
+runs it will use your editor, set-local-editor_, so you can supply ``Reviewers``
+and ``Subscribers`` in your commit message. It requires **one or more** Reviewers
+and allows **zero or more** Subscribers, finding-reviewer-names_.
+
----------------
lebedev.ri wrote:
> winksaville wrote:
> > winksaville wrote:
> > > lebedev.ri wrote:
> > > > It really should be emphasized that either the appropriate repo field should be set,
> > > > or appropriate -commits list be subscribed, from the start.
> > > I'm don't know what you mean by 'appropriate repo field'. I've emphasied there must be **one or more** Reviewers.
> > > I could explicitly say that Differential is automatically adding "llvm-commits" to Subscribers.
> > > 
> > > Or do you mean something else?
> > @lebedev.ri do you mean add information about forking of llvm/llvm-project, clone the fork and then creating a commit?
> I'm only highlighting "The mailing list should be added as a subscriber on all reviews" from https://llvm.org/docs/Phabricator.html
> I think that a similar phrase really should be repeated here.
Ok, so instead of:

"It requires **one or more** Reviewers and allows **zero or more** Subscribers, finding-reviewer-names_."

How about:

"`arc` requires **one or more** Reviewers (finding-reviewer-names_) and Subscribers should be **llvm-commits** plus the appropriate **-dev** list(s) from [listinfo](https://lists.llvm.org/mailman/listinfo)."

I'll update llvm/docs/Phabricator-commit-message.png when I get it right :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61267/new/

https://reviews.llvm.org/D61267





More information about the llvm-commits mailing list