[PATCH] D155081: Specify the developer policy around links to external resources

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 12 06:18:47 PDT 2023


aaron.ballman created this revision.
aaron.ballman added reviewers: rnk, lattner, doug.gregor, probinson, ldionne, arsenm, mehdi_amini.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

This specifies the developer policy on adding links in source & test files and commit messages, related to discussion at: https://discourse.llvm.org/t/code-review-reminder-about-links-in-code-commit-messages/71847

The intent is to discourage adding links to resources that are not available to the community as a whole (dead links, links to internal documentation, links to internal bug trackers, etc) from source and test files, while still allowing such links to appear in other contexts as needed. It suggests to instead add sufficient context in the surrounding comments to make such links unnecessary.

It also clarifies that these links can appear in commit messages as metadata (similar to how we already have Differential Revision and Fixes metadata with links).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155081

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===================================================================
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -201,6 +201,11 @@
   entire failing program into ``llvm/test`` as this creates a *time-to-test*
   burden on all developers. Please keep them short.
 
+* Avoid adding links to resources that are not available to the entire
+  community, such as links to private bug trackers, internal corporate
+  documentation, etc. Instead, add sufficient comments to the test to provide
+  the context behind such links.
+
 Note that llvm/test and clang/test are designed for regression and small feature
 tests only. More extensive test cases (e.g., entire applications, benchmarks,
 etc) should be added to the ``llvm-test`` test suite.  The llvm-test suite is
@@ -256,6 +261,11 @@
    the change (more invasive changes require more testing). A reasonable subset
    might be something like "``llvm-test/MultiSource/Benchmarks``".
 
+#. Ensure that links in source code and test files are to publicly available
+   resources and are used primarily to add additional information rather than
+   to supply critical context. The surrounding comments should be sufficient
+   to provide the context behind such links.
+
 Additionally, the committer is responsible for addressing any problems found in
 the future that the change is responsible for.  For example:
 
@@ -336,8 +346,6 @@
   code snippets and gory details should be left to bug comments, web
   review or the mailing list.
 
-* If the patch fixes a bug in GitHub Issues, please include the PR# in the message.
-
 * Text formatting and spelling should follow the same rules as documentation
   and in-code comments, ex. capitalization, full stop, etc.
 
@@ -346,8 +354,14 @@
   related commit. This could be as simple as "Revert commit NNNN because it
   caused PR#".
 
-* If the patch has been reviewed, add a link to its review page, as shown
+* It is acceptable to add metadata to the commit message to automate processes.
+  If the patch fixes a bug in GitHub Issues, we encourage adding
+  "Fixes https://github.com/llvm/llvm-project/issues/12345" to automate closing
+  the issue in GitHub. If the patch has been reviewed, we encourage adding a
+  link to its review page, as shown
   `here <https://www.llvm.org/docs/Phabricator.html#committing-a-change>`_.
+  Other kinds of metadata are also acceptable, including links to resources
+  that are not available to the entire community.
 
 For minor violations of these recommendations, the community normally favors
 reminding the contributor of this policy over reverting. Minor corrections and


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155081.539525.patch
Type: text/x-patch
Size: 2676 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230712/3416d0ee/attachment.bin>


More information about the cfe-commits mailing list