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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to