klimek added a comment. In D69764#2934032 <https://reviews.llvm.org/D69764#2934032>, @MyDeveloperDay wrote:
> In D69764#2932929 <https://reviews.llvm.org/D69764#2932929>, @steveire wrote: > >> > > @steveire, thanks for your comments, I also agree that a second tool > shouldn't be needed, especially as this functionality is off by default (and > needs to stay that way). This was my suggestion for a compromise. > > Its a tough call, but ultimately no on ever gave this a LGTM so I couldn't > proceed anyway. Whilst I think we can cover the other qualifiers with a > similar approach I don't want to waste more of my time until the fundamental > question has been answered if its ok for clang-format to insert/modify the > order of tokens and that is more a collective decision. > > It was suggested to me that I write up and RFC on the matter, I'm not a > massive fan of those as I don't really see a formal process for handling them > (the conversation seems to turn into a series of personal preferences then > dwindles and dies without conclusion). But if people think it would solve > something I guess I could try. > > From my perspective I'm more interested in the views of the major > clang-format contributors (@curdeius, @krasimir, @owenpan , @sammccall, > @HazardyKnusperkeks and people like yourselves who have put time and effort > into blogging about these tools), ultimately it will be us who have to handle > the impact of this (and maybe the #clangd people) and I don't want to > inconvenience them or make work for them. > > So here is what is needed to land this in my view: > > 1. I'd want at least 2 to 3 of the current reviewers to LGTM. > 2. I'd want 1 person (like yourself) a community contributor to also give > their approval (I guess I have that conceptually from you!) > > or > > 1. @klimek or @djasper to give us an its ok for "clang-format to > insert/modify the order of tokens" in a controlled manner decision > 2. 1 LGTM > > Whilst I respect the views of those few who have objected, they don't to my > knowledge make significant contributions to clang-format (not in the last 5-6 > years I've been following it anyway) > > I feel this team owns the decision as we are the ones that look after it, > warts and all. We'll always have a situation where some don't agree, thats > ok, but I feel there is more agreement that this funcationality is ok rather > than not. > > Until one of those 2 things happens, we are in limbo. I think we had a really good, inclusive discussion on this change, so I don't think an RFC would add anything to it. I think we have consensus on, if we want this, we: 1. want it behind an option that is not enabled in default-configs 2. we want users to understand that this has the potential to introduce bugs I also think this is a feature that seems useful enough for a large enough group, to provide it in clang-format. I'd put a lot of weight on @MyDeveloperDay's evaluation, as in the end, he's currently taking the most active role in keeping clang-format alive & well. One idea for clang-format going forward is: can we make it easier for people to understand what flags can introduce non-whitespace-changes? E.g. have flag name prefixes like semantic-*. So, I'm generally OK with this. One minor nit I have is that I'd personally add unit tests to the EastWestConstFixer, as it's quite a large class, but given that I'll duck out after this comment, I'll not insist :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
