steveire added a comment. In D69764#2050538 <https://reviews.llvm.org/D69764#2050538>, @MyDeveloperDay wrote:
> In D69764#2050226 <https://reviews.llvm.org/D69764#2050226>, @steveire wrote: > > > I like the approach of using clang-format to implement this. It's much > > faster than a `clang-tidy` approach. > > > > The broader C++ community has already chosen `East`/`West` and it has > > momentum. If you choose `Left`/`Right` now, you will get pressure to add > > `East`/`West` in the future, which means we'll have the synonyms we want to > > avoid. > > > > The broader C++ community already has understanding of `East`/`West`. > > Trying to change that now should be out of scope for this patch. This patch > > should use `East`/`West`. > > > > I ran this on a large codebase and discovered some problems with this > > patch. Given this `.clang-format` file: > > > Thank you for this feedback @steveire, to be honest I agree, I didn't want to > waste time arguing about the naming for now so simply gave in. Supporting > multiple words from the outset also felt wrong, maybe we can spin around > later towards the end of the review if there is more of a concencus on naming > being the other way. It seems to be only Aaron who is against East/West. And his objection doesn't seem to be considerate of the broader consensus. I'm sure it's no problem to use those names. > Thank you also for the failure scenarios I will add them as tests as I try to > improve this further. > > I think there was a suggestion that somehow this should cover all forms of > identifier ordering but I actually think that is going to be incredibly > complex especially if that configuration was completely dynamic and supported > custom types and macros > > For now let me pursue fixes for the cases you have identified. Great, here's a few more which don't currently get converted :) : void autofn() { const auto i = 0; const auto& ir = i; const auto* ip = &i; } 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
