+1 2015-09-10 9:44 GMT+08:00 Marco Massenzio <[email protected]>:
> +1 > > > > > Thanks, Michael! > > > > — > Sent from my iPhone, which is not as good as you'd hope to fix trypos n > abbrvtn. > > On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <[email protected]> wrote: > > > 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. > >> > >>>>> > >> > >>>>> > >> > >>> > >> > >> > >> > > >> > > >> > -- Deshi Xiao Twitter: xds2000 E-mail: xiaods(AT)gmail.com
