Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============7491442759422001182=="
MIME-Version: 1.0
Subject: Re: Review Request 39985: [1/5] Introduced ACL protobuf definitions
 for dynamic reservation.
From: "Adam B" <adam@mesosphere.io>
To: "Michael Park" <mcypark@gmail.com>, "Adam B" <adam@mesosphere.io>,
 "Jie Yu" <yujie.jay@gmail.com>, "Till Toenshoff" <toenshoff@me.com>
Cc: "Guangya Liu" <gyliu513@gmail.com>, "Greg Mann" <greg@mesosphere.io>,
 "mesos" <reviews@mesos.apache.org>
Date: Fri, 06 Nov 2015 09:05:30 -0000
Message-ID: <20151106090530.1770.69251@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Adam B" <noreply@reviews.apache.org>
References: <20151106072200.1771.29471@reviews.apache.org>
In-Reply-To: <20151106072200.1771.29471@reviews.apache.org>
Reply-To: "Adam B" <adam@mesosphere.io>

--===============7491442759422001182==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Nov. 5, 2015, 11:22 p.m., Guangya Liu wrote:
> > include/mesos/authorizer/authorizer.proto, line 28
> > <https://reviews.apache.org/r/39985/diff/1/?file=1116935#file1116935line28>
> >
> >     We have some discussions want to reduce the comments to 70 chars per line.
> 
> Adam B wrote:
>     We actually recently changed the style guide to no longer require comments to wrap at 70 chars.
>     Now comments can/should wrap at 80 chars like everything else. We will not do a sweeping change.
> 
> Guangya Liu wrote:
>     I see that we are still under discussion for this in mail list, seems @bmahler has some different comments, please search "Mesos Style Guideline Adjustments?" in dev list for detail.
>     
>     Just cite the comments from bmahler from dev list:
>     This has come up in a couple of reviews, seems like we should add some soft
>     guidelines around how to format comments for readability.
>     
>     In particular, the reason that we wrapped at 70 in the past was for
>     readability, so it would be great to continue doing so as a soft stylistic
>     rule. The other thing we've been doing for readability is reducing
>     "jaggedness" (variability in line lengths).
>     
>     It would be great to establish these as soft rules and encourage new
>     contributors / committers to follow them. Compare these two comments in
>     Master::updateTask. The first one wraps at 70 and reduces jagedness, the
>     second wraps at 80 and is more jagged:
>     
>     https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
>     https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>     
>     I can provide more examples to help clarify. If no one objects, I'll follow
>     up with an update to the style guide. Thoughts appreciated!
> 
> Guangya Liu wrote:
>     You can also refer here: http://search-hadoop.com/m/0Vlr6MlNkc1FGzCj2/Mesos+Style+Guideline+Adjustments%25E2%2580%258F+Ben+Mahler+This+has+come+up+in+a+couple+of+reviews%252C+seems+like+we+should+add+some+soft&subj=Re+Mesos+Style+Guideline+Adjustments
>     
>     This is still under discussion.

Fair enough, but if the goal is to reduce jaggedness, that's yet another argument for keeping this comment all on one line, even past 70 chars.
(Thanks for the discussion reference. I knew that we'd recently removed the 70 char restriction, but wasn't abreast of the ongoing discussion.)


- Adam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39985/#review105376
-----------------------------------------------------------


On Nov. 5, 2015, 1:50 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39985/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2015, 1:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
>     https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduced ACL protobuf definitions for dynamic reservation.
> Note: this review is continued from https://reviews.apache.org/r/37002/
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
> 
> Diff: https://reviews.apache.org/r/39985/diff/
> 
> 
> Testing
> -------
> 
> This is the first in a chain of 5 patches. `make check` was used to test at the end of the chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


--===============7491442759422001182==--