+1 > On Sep 10, 2015, at 4:21 PM, tommy xiao <xia...@gmail.com> wrote: > > +1 > > 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>: > >> +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. >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>> >>>>> >>>>> >>>> >> > > > > -- > Deshi Xiao > Twitter: xds2000 > E-mail: xiaods(AT)gmail.com
signature.asc
Description: Message signed with OpenPGP using GPGMail
