mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gastón Kleiman <gas...@mesosphere.io>
Subject Re: Review Request 56601: Used single quotes instead of backticks in error message.
Date Thu, 16 Feb 2017 13:48:25 GMT


> On Feb. 14, 2017, 1:48 p.m., Gastón Kleiman wrote:
> > I think that we should also update `checks/checker.cpp` and `checks/health_checker.cpp`:
> > 
> > ```
> > $ ag --cpp --nomultiline '^\s*[^/"]+".*`'
> > checks/checker.cpp
> > 60:            "Check's `CommandInfo` is invalid: " + error->message);
> > 
> > checks/health_checker.cpp
> > 1111:            "Health check's `CommandInfo` is invalid: " + error->message);
> > ```
> 
> Benjamin Bannier wrote:
>     I didn't intend to make a full sweep in this patch, but rather to address a single
such instance in the reservation validation code I touched.
>     
>     I believe a full sweep should fix a much larger number of places (in addition to
error messages probably also in e.g., help strings),
>     
>         $ git grep -n '".*`' | grep -v '^.*\/\/' | grep -E '(cpp:|hpp:)' | wc -l
>              328
> 
> Benjamin Mahler wrote:
>     Wow, I did not realize the extent to which backticks have made it into our logging,
error, and help messages. This would warrant a thread on the dev list to make sure everyone
is on the same page, because from what I can tell it has become inconsistent. I'm curious
to know if there were any reasons to also include backticks in logging, error, and help messages
(other than being consistent with comment formatting style).

I agree that it has become inconsistent and that we'd probably require a dev thread and a
full sweep to make all the logging, error, and help messages consistent.

However I think that making all the validation errors consistent wouldn't be so hard. As far
as I can see, it'd require only to fix the 4 issues I opened here and to update `checker.cpp`
and `health_checker.cpp`, which I would volunteer to do.


- Gastón


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


On Feb. 16, 2017, 10:24 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56601/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f 
>   src/tests/master_validation_tests.cpp 83c88ae3a1f54eab29de79c0f34ade0cdb410341 
> 
> Diff: https://reviews.apache.org/r/56601/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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