[PATCH] D87587: [clang-format][PR47290] Make one-line namespaces resistant to FixNamespaceComments, update documentation

Mon Sep 14 10:29:43 PDT 2020

kuzkry added a comment.

Hey @MyDeveloperDay, thanks for the review.

On the one hand, you're right, it's a breaking change and I dislike it too.
But please, reconsider because on the other hand:
a) adding a new option means increasing our maintenance cost by possibly adding a rarely-used switch (which I also dislike)
b) it's been a part of clang-format documentation since release 5 so we could make clang-format work as expected
c) this patch doesn't have to be applied to already released versions and could be merged with a new major release. I think people can expect certain things to work differently while changing major versions.
d) in general, people care about preserving their git history and they normally don't format all their code blindly. I think the most frequent use case it to format only adjacent parts of code.

So, I'd say both ways have their ups and downs.

@all
Still, so far I've got two change requests from you lot but they are mutually exclusive.
To sum it up, which do you prefer?
a) add a new option (@MyDeveloperDay's approach)
b) change kShortNamespaceMaxLines (my approach) assuming @JakeMerdichAMD's suggestions (non-empty one-line namespaces aren't given the end comment)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87587/new/

https://reviews.llvm.org/D87587