I've removed the 70 column restriction on comments from the style guide: https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
Also, based on the comments, it seems like we should allow 80 column comments but omit the sweeping change. Thanks, MPark. On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <[email protected]> wrote: > On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <[email protected]> > wrote: > > > Like BenM, > > > > +1 on allowing 80 column comments > > > +1 > (it really IS annoying having to keep an eye on the bottom column counter > when typing comments :) > > > > -1 on sweeping changes; incremental changes when touching old comments > > will do IMHO > > > > +1 on the -1? :) > Incremental changes are good and I doubt anyone will be "confused" by them. > > > > Bernd > > > > > On Aug 12, 2015, at 12:51 AM, Michael Park <[email protected]> wrote: > > > > > > 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 < > > [email protected]> > > > 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 < > > [email protected]> > > >> 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 <[email protected]> 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 < > > >> [email protected]> > > >>>> 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 <[email protected]> > > >> 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 <[email protected] > > > > >>> 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 <[email protected]> > > >>> 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 < > > >>>>>>> [email protected]> > > >>>>>>>> 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 < > [email protected] > > > > > >>>>>>> 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. > > >>>>> > > >>>>> > > >>> > > >> > > > > >
