[PATCH] D155081: Specify the developer policy around links to external resources
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 15 08:34:11 PDT 2023
reames added inline comments.
================
Comment at: llvm/docs/DeveloperPolicy.rst:349
-* If the patch has been reviewed, add a link to its review page, as shown
- `here <https://www.llvm.org/docs/Phabricator.html#committing-a-change>`_.
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > reames wrote:
> > > Removing this item seems very off topic for the change description, and certainly hasn't been discussed in the linked thread. Please add this back in a separate commit.
> > >
> > > (To be clear, no objections to the overall change, just the removal of the phab link text.)
> > Hmm, I thought this was obsoleted by the new text (it is covered by "other kinds of metadata"). That said, losing that link is definitely a regression, so thank you for pointing this out! I'll find a way to add it back in (either as a stand-alone bullet point or incorporated into the new text).
> I restored the link in https://github.com/llvm/llvm-project/commit/a1562bbc63b49a70b39ba075d9a3332f50cea11d as part of the new bullet; please let me know if you have additional concerns.
That 90% covers it. What's left is some minor framing. I'd suggest separating that point into two. The first should be recommended metadata (phab, issues link), and the second can be the additional metadata point. Something like:
```
If the patch has been reviewed, add a link to its review page, as shown
`here <https://www.llvm.org/docs/Phabricator.html#committing-a-change>`_. If the patch fixes a bug in GitHub Issues, we encourage adding a reference to the issue being closed, as described `here <https://llvm.org/docs/BugLifeCycle.html#resolving-closing-bugs>`_.
It is also acceptable to add other metadata to the commit message to automate processes, including for downstream consumers. and including links to resources that are not available to the entire community. However, such links and/or metadata should not be used in place of making the commit message self-explanatory.
```
All of the above is just reorganizing what you had written with some very minor copy editing. I'd separately suggest adding the following sentence at the end of the second bullet.
Note that such non-public links are *only* allowed in commit messages, and should not be included in the submitted code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155081/new/
https://reviews.llvm.org/D155081
More information about the llvm-commits
mailing list