[PATCH] D28184: [doc] Clarify steps for contributors without commit access.

Anmol P. Paralkar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 30 14:25:49 PST 2016


anmol added a comment.

@fhahn thank you, this is definitely helpful to a new contributor to understand the process.

I note the wording: "...it can then be committed to trunk" and later on: "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." So, ultimately, the contributor needs to indicate whether they wish for their patch to get committed? I would think that putting a patch on Phabricator implies that ultimately when all looks good about it, the intent is that it gets committed to trunk. On the other hand, for a reviewer with commit access, do they or do they not assume that the patch is meant to be committed? So, that needs to be clarified.

Perhaps, when the state of the review changes to: "This revision is now accepted and ready to land.", we have two questions come up for the contributor to answer:

[ ] Do you wish for this patch to be committed to trunk?

(if so) [] Do you have commit access?

- that way, the fate of the patch does not remain ambiguous.

Also, if I may suggest:

[line 133]  indicating you cannot commit the patch -> indicating that you cannot commit the patch
 [line 135] recommend -> recommended

PS: I tried to add myself as a reviewer to this review, but could not figure it out. (I read: http://llvm.org/docs/Phabricator.html as well as tried to play with the controls on the top right).


https://reviews.llvm.org/D28184





More information about the llvm-commits mailing list