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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
