> On May 9, 2015, 2:27 a.m., Marco Massenzio wrote:
> > include/mesos/executor/executor.hpp, line 22
> > <https://reviews.apache.org/r/33823/diff/1/?file=949197#file949197line22>
> >
> >     please avoid SCREAMING COMMENTS :)
> >     (also, not sure what is the use of that comment? is it a TODO? should 
> > we have a #pragma? what?)
> 
> Alexander Rojas wrote:
>     1. It is a warning.
>     2. The purpose of this header is to be a reflection of 
> `include/mesos/scheduler/scheduler.hpp`, and so they both have the same 
> comments, in fact they look mostly identical which follows the maxim of being 
> consistent.

Well, two wrongs don't make one right.
Maybe fix both, or at least let's not perpetuate something that maybe was not 
ideal to start with.

Please change to something that reads like:

// NOTE: this header only becomes valid after running protoc 
// and generating the equivalent .ph.h files
// See: src/messages/mesos.proto
(or whatever the proto file is)

or something to that effect.
SCREAMING COMMENTS ARE BAD FORM !!!!!! 
:-)

(you could almost hear me screaming, could you? :)))


> On May 9, 2015, 2:27 a.m., Marco Massenzio wrote:
> > include/mesos/executor/executor.proto, line 80
> > <https://reviews.apache.org/r/33823/diff/1/?file=949198#file949198line80>
> >
> >     `message` is becoming a truly overloaded term here: as a Protobuf 
> > message, the Message class, this (string) variable here, the (Message) 
> > `message` field below... how about `errorMessage` instead?
> 
> Alexander Rojas wrote:
>     It is called like that, because it is forwarded from the master and to 
> the master. While I agree with you in principle, you know we like 
> consistency. Why not discuss it in the HTTP API meeting?

I don't understand the "consistency" reference.  This is an `error message` and 
should be named as such? what is not "consistent" about that?
When we use the same term with (semantically) different meanings, we make our 
code more difficult to read and understand - people have to go back and forth - 
especially when, as in this case, comments are very few and far between.


- Marco


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


On May 19, 2015, 12:19 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33823/
> -----------------------------------------------------------
> 
> (Updated May 19, 2015, 12:19 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor/executor.hpp PRE-CREATION 
>   include/mesos/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
> 
> Diff: https://reviews.apache.org/r/33823/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to