mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Toenshoff" <toensh...@me.com>
Subject Re: Review Request 35701: Minor formatting cleanup in the Master.
Date Tue, 23 Jun 2015 00:56:07 GMT


> On June 22, 2015, 6:26 p.m., Till Toenshoff wrote:
> > src/master/master.cpp, lines 972-975
> > <https://reviews.apache.org/r/35701/diff/1/?file=989004#file989004line972>
> >
> >     I understand this was updated due to some discussions on related RRs. I would
however love to see the definitive answer to https://issues.apache.org/jira/browse/MESOS-2618
for preventing any subjective back and forth. The key-question here is "when are we reaching
the jaggedness problem"?
> >     
> >     Would you mind adding a comment to the ticket with your recent findings and
experience from those other RRs?
> 
> Ben Mahler wrote:
>     Hey Till, the particular case Michael fixed here was bad alignment: it doesn't fit
any of the options in the style guide. So I doubt there would be any controversy here :)
> 
> Till Toenshoff wrote:
>     Thanks for chiming in Ben. While I see that the original formatting was off in any
case, I still see a potential problem with this variable indentation for the argument lists.
>     
>     For fixing the above, according to our current styleguide, MPark could have chosen
any of the below.
>     
>     A. Variable Indentation:
>     ```
>     delay(failoverTimeout,
>           self(),
>           &Master::frameworkFailoverTimeout,
>     	  framework->id(),
>     	  framework->reregisteredTime);
>     ```
>     
>     B. Fixed Indentation:
>     ```
>     delay(
>         failoverTimeout,
>         self(),
>         &Master::frameworkFailoverTimeout,
>     	framework->id(),
>     	framework->reregisteredTime);
>     ```
>     
>     However, how to be able to assert that A. would be fine even though we try to avoid
"jaggedness" is a real problem to me. I wondered if we could get closer to that "definitive
answer". Any input is greatly appreciated :)
> 
> Michael Park wrote:
>     Hey Till, I've captured my thoughts on this topic on the [JIRA ticket](https://issues.apache.org/jira/browse/MESOS-2618?focusedCommentId=14596770&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14596770).

Terrific, thanks MPark! For now, I shall drop that issue given that you payed the toll :D.


- Till


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


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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