mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joerg Schad <jo...@mesosphere.io>
Subject Re: Review Request 45863: Updated error messages in weights handler.
Date Thu, 07 Apr 2016 11:07:31 GMT

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


Fix it, then Ship it!





src/master/weights_handler.cpp (line 90)
<https://reviews.apache.org/r/45863/#comment190984>

    Not yours: Above the request.body is included in the output. Feels inconsistent here...



src/master/weights_handler.cpp (line 94)
<https://reviews.apache.org/r/45863/#comment190982>

    Could you add a similar comment as in quota_handler (// Check that the role is on the
role whitelist, if it exists.). IMO for isWhitelistedRole it is not  obvious from the name
that it will be true if there is no whitelist.



src/master/weights_handler.cpp (line 97)
<https://reviews.apache.org/r/45863/#comment190983>

    Not yours, still: This feels a little off as it must only exist in the whitelist if there
is a whitelist specified.


- Joerg Schad


On April 7, 2016, 10:50 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45863/
> -----------------------------------------------------------
> 
> (Updated April 7, 2016, 10:50 a.m.)
> 
> 
> Review request for mesos, Yongqiao Wang and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> While writing log or error messages we adhere the following style:
>   * No period at the end (there are exceptions though, e.g., one is
>     when constructing request responses.
>   * When splitting a string over multiple lines, put a space at the
>     beginning of the following line in contrast to the end of the
>     previous line.
> 
> 
> Diffs
> -----
> 
>   src/master/weights_handler.cpp e88bf2ab67ccadf35879b92f3280298a43d7cd0e 
> 
> Diff: https://reviews.apache.org/r/45863/diff/
> 
> 
> Testing
> -------
> 
> On Mac OS 10.10.4: 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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