klimek added a comment. In D33029#1423949 <https://reviews.llvm.org/D33029#1423949>, @MyDeveloperDay wrote:
> In D33029#1423947 <https://reviews.llvm.org/D33029#1423947>, @lebedev.ri > wrote: > > > In D33029#1423944 <https://reviews.llvm.org/D33029#1423944>, > > @MyDeveloperDay wrote: > > > > > Adding the unit tests lets us see how this option will work in various > > > cases, it will let us understand that its not breaking anything else. > > > > > > I personally don't like to see revisions like this sit for 2 years with > > > nothing happening, I don't see anything wrong with this that would > > > prevent it going in so I don't understand whats blocking it?, > > > > > > if you had some tests and a release note I'd give it a LGTM (but as I've > > > said before I'm not the code owner, but someone wanting to address > > > defects and add capabilities. but I think we need to be able to move > > > forward, people will object soon enough if they don't like it.) > > > > > > Generally I don't understand why clang-format is so reluctant to change > > > anything, As such we don't have many people involved and getting anything > > > done (even defects) is extremely hard. > > > > > > It looks like you met the criteria, and reviewers have been given ample > > > opportunity to make an objection. the number of subscribers and like > > > tokens would suggest this is wanted, > > > > > > Please also add a line the in the release notes to say what you are > > > adding. In the absence of any other constructive input all we can do is > > > follow the guidance on doing a review, for what its worth I notice in the > > > rest of LLVM there seems to be a much larger amount of commits that go in > > > even without a review, I'm not sure what makes this area so strict, so > > > reluctant to change especailly when revisions do seem to be reviewed. > > > > > > I don't have any stake here, but i just want to point out that no tool > > (including clang-format) > > will ever support all the things all the people out there will want it to > > support. But every > > new knob is not just a single knob, it needs to work well with all the > > other existing knobs > > (and all of the combination of knob params), and every new knob after that. > > > > It's a snowball effect. Things can (and likely will, unless there is at > > least a *very* strict testing/quality policy > > (which is > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options > > about)) > > get out of hand real quickly.. > > > > Just 2c. > > > Correct we should always be adding tests and show we don't break existing > tests... We need to apply the "Beyonce Rule".. "if you liked it you should > have put a test on it." > > We shouldn't just give up making improvements or fixing defects because > something is hard or complex. The problem is that clang-format is already at a complexity level where most patches for new things we see are overly complex and will make adding new things *even harder*, reducing the ability to move at all. As I've written before, I agree that we're in a bad state, and I'm truly sorry for it - we need to get active contributors into a state where they can make changes aligned with the architectural spirit of clang-format, which make clang-format at least not more complex :) For this specific patch, yes, please change it as djasper suggested to be configurable in the BracketAlignmentStyle. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33029/new/ https://reviews.llvm.org/D33029 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
