Ben, thanks for your input! Another update on this topic: the patches around break before braces for *enum* style and overloaded operators have been committed.
On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <benjamin.mah...@gmail.com> wrote: > We already don't necessarily wrap at 70 characters (often we wrap before 70 > to reduce "jaggedness" or to make it look cleaner). > > So with the change to 80, this still makes all existing comments valid. We > can still encourage folks to write paragraphs in a way that is easy to > digest for the reader. That is, I think we should still be trying not to > write jagged paragraphs of comments, it's just not a hard stylistic > violation given we don't have an algorithm for this. > > So +1 to relaxing the hard 70 character rule, but -1 to sweeping across all > the comments or doing wrapping based only on line length rather than > jaggedness going forward. > > On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <jo...@mesosphere.io> > wrote: > > > I will volunteer to update all the comments to wrap at 80 if we agree to > > keep the code base consistent. > > Naturally that is also my vote ;-) > > Joris > > > > > On Aug 8, 2015, at 1:40 PM, Michael Park <mcyp...@gmail.com> wrote: > > > > > > An update on this topic since we covered it at the community developer > > sync. > > > > > > 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as their style > > is > > > equivalent to ours. The only change this entails for our codebase is > to > > > consistently wrap the braces for *enum* definitions, as we're > currently > > > inconsistent. I've taken on the work involved in this change: > > > - stout: https://reviews.apache.org/r/37258 > > > - libprocess: https://reviews.apache.org/r/37259 > > > - mesos: https://reviews.apache.org/r/37260 > > > 2. We will drop the rule for adding spaces around overloaded > > > operators. We'll simply do a sweep of the codebase to update all of > > them > > > consistently. Artem has kindly taken action on this already: > > > - stout: https://reviews.apache.org/r/37018/ > > > - libprocess: https://reviews.apache.org/r/37017/ > > > - mesos: https://reviews.apache.org/r/37013/ > > > 3. We will drop the rule for wrapping comments at 70 characters. > We > > > have a few options to proceed here: > > > - Keep all the existing comments in tact, and simply allow new > > > comments to wrap at 80, this is less work. > > > - Update all instances of the comments wrapping at 70 to be > wrapped > > > at 80, so that we can be consistent. > > > > > > I proposed that we simply allow new comments to wrap at 80, but I have > > > heard arguments to update the existing comments, so that we can be > > > consistent across the codebase. If you have a suggestion/opinion on how > > we > > > should proceed with (3), please share! > > > > > > Thanks, > > > > > > MPark. > > > > > > On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas < > alexan...@mesosphere.io> > > > wrote: > > > > > >> I also vote up for that! I rather change our guidelines a little bit > > than > > >> waiting for months > > >> to get our changes into the clang-format source without any security > > that > > >> it will actually land. > > >> > > >>> On 31 Jul 2015, at 09:31, Alex Rukletsov <a...@mesosphere.com> > wrote: > > >>> > > >>> I think automation is very important. If we should slightly change > our > > >>> style in order to set-up easily enforceable rules, I vote with both > > hands > > >>> for that. > > >>> > > >>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <mcyp...@gmail.com> > > wrote: > > >>>> > > >>>> Oops, sorry I was so excited that this could just solve the issue > > that I > > >>>> forgot to answer your question. > > >>>> > > >>>> In general, the clang-format strives to adopt widely used styles, > > which > > >> I'm > > >>>> not sure if we would be considered widely used. Aside from that, > > another > > >>>> concern was that it could take a while for our style proposals to > make > > >> it > > >>>> upstream and for it to be useful. > > >>>> > > >>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <mcyp...@gmail.com> > > wrote: > > >>>>> > > >>>>> Is it worth adding our own style? > > >>>>> > > >>>>> > > >>>>> > > >>>>> I noticed other have (LLVM, Google, Chromium, Mozilla, WebKit.). > How > > >>>>>> hard is it? > > >>>>> > > >>>>> > > >>>>> I was just looking into this again and *Mozilla* was added as the > > >> newest > > >>>>> *BreakBeforeBraces* style. It breaks before braces on enum, > function, > > >> and > > >>>>> record definitions (struct, class, union). I think we can actually > > use > > >>>> that > > >>>>> one and be done with it. Having looked through the codebase, we > wrap > > >> the > > >>>>> braces for *enum* for about half of the cases. It would be about 35 > > >>>>> instances that we have to fix from what I can see in our codebase. > > What > > >>>> do > > >>>>> you think? > > >>>>> > > >>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler < > > >>>> benjamin.mah...@gmail.com> > > >>>>> wrote: > > >>>>> > > >>>>>> Is it worth adding our own style? > > >>>>>> > > >>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla, WebKit.). > How > > >>>> hard > > >>>>>> is it? > > >>>>>> > > >>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <mcyp...@gmail.com> > > >>>> wrote: > > >>>>>> > > >>>>>>> There are a few syntactical Mesos style guidelines that I would > > like > > >>>> to > > >>>>>>> propose to drop. They are: > > >>>>>>> > > >>>>>>> 1. Open braces for namespace should not be wrapped. > > >>>>>>> *NOTE*: The Google style guide does not wrap the brace after > > >>>>>>> *namespace*, > > >>>>>>> and the Mesos style guide does not mention a rule at all. But it > > is > > >>>>>>> consistent throughout the codebase. > > >>>>>>> 2. Overloaded operators should be padded with spaces. > > >>>>>>> 3. Comments should be wrapped at 70 characters. > > >>>>>>> > > >>>>>>> The main motivation is that as a community we would like to > reduce > > >> the > > >>>>>>> discrepancy between what *clang-format* produces. This is a dual > > >>>>>> effort, as > > >>>>>>> we work on improving *clang-format* to support some of our style > > >> which > > >>>>>> is > > >>>>>>> popular in the C++ community as well. Wrapping the function > > arguments > > >>>> to > > >>>>>>> avoid "jaggedness" for example is a feature request which is > being > > >>>>>> tracked > > >>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422 > > >>>>>>> > > >>>>>>> Going forward, the proposal is to update all of the instances of > > (1) > > >>>> and > > >>>>>>> (2) at once. For (3), we can simply relax the constraint from 70 > to > > >> 80 > > >>>>>>> without touching the existing comments. > > >>>>>>> > > >>>>>>> Does anyone have any strong opinions about dropping any of the 3 > > >> rules > > >>>>>>> above? > > >>>>>>> > > >>>>>>> Thanks, > > >>>>>>> > > >>>>>>> MPark. > > >> > > >> > > >
