Like BenM, +1 on allowing 80 column comments -1 on sweeping changes; incremental changes when touching old comments will do IMHO
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. >>>>> >>>>> >>> >>
