+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 <mcyp...@gmail.com> 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 <ma...@mesosphere.io> wrote: >> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <be...@mesosphere.io> >> 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 <mcyp...@gmail.com> 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 < >> > 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. >> > >>>>> >> > >>>>> >> > >>> >> > >> >> > >> > >>
