mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guangya Liu" <gyliu...@gmail.com>
Subject Re: Review Request 39985: [1/5] Introduced ACL protobuf definitions for dynamic reservation.
Date Fri, 06 Nov 2015 08:56:12 GMT


> On 十一月 6, 2015, 7:22 a.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.

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


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


On 十一月 5, 2015, 9:50 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39985/
> -----------------------------------------------------------
> 
> (Updated 十一月 5, 2015, 9: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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message