MyDeveloperDay added inline comments.

================
Comment at: clang/lib/Format/Format.cpp:1699
     ChromiumStyle.DerivePointerAlignment = false;
+    ChromiumStyle.InsertBraces = true;
     if (Language == FormatStyle::LK_ObjC)
----------------
owenpan wrote:
> aaron.ballman wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > This is an code modifying feature, we agreed that all code modifying 
> > > > features would be off by default, opt in only
> > > Now the question arises if //default// simply only applies to 
> > > `LLVMStyle`, since that's the //default// when nothing is stated, or if 
> > > other styles are free to enable such features in their style //by 
> > > default//.
> > > 
> > > I'd say if chromium wants to do that, they should be allowed to.
> > The community reacted pretty strongly to clang-format mutating code in ways 
> > that may change the meaning of code unless there is an explicit opt-in. The 
> > reason for that is because the opt-in + documentation is what informs the 
> > user that the feature may break their code, so removing that opt-in for the 
> > Chromium style means those users have no idea about the dangers. (In 
> > general, users take a dim view of a formatting tool that breaks code.)
> > 
> > Personally, I think if the Chromium *project* wants that to be the default, 
> > they can use .clang-format files in their version control to make it so, 
> > but I don't think the Chromium *style* built into clang-format should allow 
> > it by default because that may be used by a wider audience than just 
> > Chromium developers. Basically, I think we want to be conservative with 
> > formatting features that can potentially break code (once we start breaking 
> > user code with a formatting tool, that tool gets pulled out of affected 
> > people's CI pipelines pretty quickly, which I think we generally want to 
> > avoid).
> I'm with @MyDeveloperDay and @aaron.ballman on this.
I personally would feel quite uncomfortable about going against what we agreed 
in the RFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147969

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

Reply via email to