> On Sept. 29, 2014, 1:17 p.m., Benjamin Hindman wrote: > > support/clang-format, line 6 > > <https://reviews.apache.org/r/26069/diff/2/?file=708122#file708122line6> > > > > This is actually not "good", since the '{' is not on a newline. I guess > > this is just for checking the access modifier offset, but it could be > > counter-intuitive for people.
Thanks for noticing this, I've updated the style and naming convention issues in the code samples. > On Sept. 29, 2014, 1:17 p.m., Benjamin Hindman wrote: > > support/clang-format, line 33 > > <https://reviews.apache.org/r/26069/diff/2/?file=708122#file708122line33> > > > > We haven't had a hard rule about this, so if someone manually aligns > > them where that might make sense will ClangFormat un-align them? Yes, ClangFormat will unalign them if asked. If we run `clang-format -i filename`, then ClangFormat formats the entire file. Since there are cases like this where we may want/need to format code manually, the recommended usage would be to integrate ClangFormat into your editor. With ClangFormat integrated into your editor, you can select parts of the code that you wish to format. > On Sept. 29, 2014, 1:17 p.m., Benjamin Hindman wrote: > > support/clang-format, line 56 > > <https://reviews.apache.org/r/26069/diff/2/?file=708122#file708122line56> > > > > Can we fine-tune this? We break for function, class, struct, etc, but > > not for namespaces. This is one of the 4 limitations I've outlined in the [Google Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing). There's no option to fine-tune this in the current version of clang, but I have a [fork of clang](https://github.com/mpark/clang/tree/mesos) which supports `Mesos` as one of the `BreakBeforeBraces` options. It also covers 2 other limitations: 1. Arguments should be indented by 4 spaces rather than 2 spaces. 2. Overloaded operators should be padded with spaces. The one I don't have implemented is the one you mentioned above, which is avoiding "jaggedness" in the code. I don't think it makes sense to expect people to run a custom clang-format. Having said that, I use the custom clang-format myself and am happy to help others get it working if they're interested. > On Sept. 29, 2014, 1:17 p.m., Benjamin Hindman wrote: > > support/clang-format, line 51 > > <https://reviews.apache.org/r/26069/diff/2/?file=708122#file708122line51> > > > > Can ClangFormat differentiate when to do: > > > > allocator->resourcesRecovered( > > frameworkId, > > slaveId, > > resources, > > filters); > > > > instead of: > > > > allocator->resourcesRecovered(frameworkId, > > slaveId, > > resources, > > filters); No, I don't believe so :( - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26069/#review54806 ----------------------------------------------------------- On Sept. 29, 2014, 6:33 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26069/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2014, 6:33 p.m.) > > > Review request for mesos, Benjamin Hindman, Cody Maloney, Dominic Hamon, and > Timothy Chen. > > > Bugs: MESOS-1291 > https://issues.apache.org/jira/browse/MESOS-1291 > > > Repository: mesos-git > > > Description > ------- > > We spend way too much of our time formatting, not to mention the amount of > time wasted during the review cycle to address style issues. Let’s get > ClangFormat to help us! > > If you don't know what ClangFormat is or how it works, take a look at the > Chandler Carruth's [live > demo](http://www.youtube.com/watch?v=uvddFPavYZQ#t=23m28s) from 23m 28s to > 33m 27s. > > Further details and analysis are available > [here](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing). > > > Diffs > ----- > > .clang-format PRE-CREATION > support/clang-format PRE-CREATION > > Diff: https://reviews.apache.org/r/26069/diff/ > > > Testing > ------- > > Refer to the __Sample Diff__ section in the [Google > Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing) > > > Thanks, > > Michael Park > >
