[llvm][docs] Reorder Stacked PR approaches in GitHub.rst #138126
Conversation
The 'user branches' approach now appears before the 'dependency note' approach, as it makes reviewing easier. Add notes on requiring commit access for the former and on the disadvantage of the latter.
nit: Removed a space before a dot at the 'Commit Access' link.
llvm/docs/GitHub.rst
Outdated
| This approach requires | ||
| `Commit Access <https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access>`_. | ||
| However, if you are involved with a Stacked PR, there's a good chance you'll | ||
| be granted access. |
There was a problem hiding this comment.
[nit] The paragraph you linked discusses the process of obtaining commit access, rather than providing a general definition of what "commit access" means. I suggest updating the link title to reflect that more accurately.
Also, regarding the sentence:
However, if you are involved with a Stacked PR, there's a good chance you'll be granted access.
This phrasing could be misleading. In the LLVM project, needing stacked PRs is not itself a sufficient criterion for being granted commit access. We should avoid suggesting a causal link where there isn’t one.
I suggest keeping this simple and factual: "This approach requires commit access: ".
I hope that this makes sense.
There was a problem hiding this comment.
Hey Andrzej,
Good points, and thanks for your review!
I've kept the link and now it refers on how to obtain commit access. – or do you think it's better to remove it?
There was a problem hiding this comment.
LGTM, thanks!
Perhaps wait one more day before landing, just in case @boomanaiden154 has further comments (all other active reviewers have already approved).
paschalis-mpeis
deleted the
users/paschalis-mpeis/github-doc-stacked-prs-order
branch
The 'user branches' approach now appears before the 'dependency note' approach, as it makes reviewing easier. Add notes on requiring commit access for the former approach.
The 'user branches' approach now appears before the 'dependency note' approach, as it makes reviewing easier.
Add notes on requiring commit access for the former and on the disadvantage of the latter.