[Code Review] Reminder about links in code & commit messages

For the years I was code owner of Clang, the general policy was that links into internal bug trackers were acceptable in test cases and commit messages, but should be avoided elsewhere. The reasoning was as follows: with test cases, the test case itself is usually self-explanatory, but having the link in there is useful for tracking the impact the bug had for whatever vendor found and fixed it. For commit messages, it’s helpful metadata for tooling. It is not reasonable to expect that these bugs be mirrored in LLVM’s bug tracker, because any information beyond the reduced test case will often involve confidential information, from the vendor itself or one of its customers. There’s no merit in pursuing that as a policy.

Having links to internal bug trackers in non-test code is problematic because it is too easy to use it as an excuse to not properly document the code itself. And even if you have also documented the code properly, a reader will always be left to wonder—is there something more behind that mysterious link? So our convention was not to allow them in code, because code needs to be fully explained to everyone.

I still believe that the unofficial policy we followed while I was code owner is the most reasonable one, and that this newer, stricter policy against any links to internal bug trackers is unhelpful. It doesn’t add clarity, but it does suppress useful information.

More importantly, I think this was managed extremely poorly. The original post in this thread stated a policy that differs from historical norms, was barely discussed, and is not written down anywhere. It received immediate pushback. Then within two days, the policy was retroactively enforced on a code base with a 15-year history (removing a nontrivial amount of information), attempting to change the conversation to this:

The commit is already in […] we could consider reverting select files back to their original state

It is actively hostile to land such a sweeping change amid existing objections. If you want to push for a policy change here, bring it up as a discussion, but do not commit the change and then pretend that the onus is now on others to argue for individual bits to be restored. If you truly believe that this is the right policy, then it should be an LLVM-wide policy rather than a Clang-specific one. The LLVM community appreciates its code owners and gives them broad license to make the right technical decisions for the project, but it also expects them to listen to the community, take their feedback seriously, and discuss significant changes before they are made. That did not happen here.

Doug Gregor
Clang Code Owner, Emeritus

11 Likes