> On May 22, 2015, 8:50 p.m., Marco Massenzio wrote:
> > include/mesos/slave/oversubscription.proto, line 34
> > <https://reviews.apache.org/r/34581/diff/2/?file=969904#file969904line34>
> >
> >     please consider calling this `QosCorrectiveAction`
> >     (we require CamelCase for our types, in any event; so this would have 
> > to be `QosCorrection`)
> >     
> >     I'm also not wild about the `QoS` moniker - I'd like this to be a more 
> > generic `CorrectiveAction` message, but happy to go with whatever else 
> > others suggest.
> 
> Jie Yu wrote:
>     I prefer QoSCorrection since QoS is an abbreivation. THis is similar to 
> SlaveID and we don't callid SlaveId :)
> 
> Niklas Nielsen wrote:
>     +1
> 
> Marco Massenzio wrote:
>     For the record, SlaveID is a violation of the Google Style guide we 
> purportedly follow - it's probably too late to fix now, but we should avoid 
> to perpetuate the mistake.
>     
>     The rationale of the rule is that it does not leave room for guessing the 
> uppercase/lowercase: it's strictly CamelCase (so, HttpServer or UuidFactory 
> or DeaEnforcingAgent....)
>     
>     As it is, `QoSCorrection` violates the style guide; please don't do this.
> 
> Bartek Plotka wrote:
>     ID in SlaveID is 2 characters long and this can be "legally" put all in 
> caps. ((: 
>     
>     On the other hand, would you prefer QosCorrection than QoSCorrection? In 
> my opinion the latter looks fine (rather everybody knows that *QoS* is the 
> abbreviation), even that we may violate some style guides (e.g Microsoft 
> one). What matters here is what looks better (more understandable) and that 
> depends on someone's opinion.. 
>     
>     +1 for consistency with SlaveID, but i agree that kind of violates the 
> common guidelines - maybe we should define it explicitly in 
> http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ ? 
>     
>     Does it make sense?

Where did you derive the impression that two-letter all-uppercase are fine?
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Type_Names

Exactly because "what looks better [...] depends on someone's opinion" we 
didn't leave room for interpretation: it's CamelCase, simple as that.
And having a rule that states the equivalent of "Type Names are all CamelCase, 
unless you don't like it, in which case it's fine to use whichever case looks 
better" would be a bit of a challenge, don't you think? :)

Again, I'm not one of the committers, so if they are happy with this, I'll go 
along with it.

BTW - just FYI: ID is not the abbreviation for I.D. (Identification Document), 
it's the abbreviation for `Identifier` so the two-letter capital case is just 
plain wrong (but very widely used).  Funnily enough, 
[UUID](http://en.wikipedia.org/wiki/Universally_unique_identifier) should have 
actually been UUId, but, hey, whatever :)


- Marco


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


On May 26, 2015, 10:24 p.m., Bartek Plotka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34581/
> -----------------------------------------------------------
> 
> (Updated May 26, 2015, 10:24 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Niklas Nielsen, 
> Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2760
>     https://issues.apache.org/jira/browse/MESOS-2760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This proto describes a QoS correction message for particular executor or task.
> It is a generic message between QoS Controller and slave.
> 
> Additionaly, updated Makefile to include this proto during compilation.
> 
> This request updates the https://reviews.apache.org/r/34571/
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/oversubscription.proto PRE-CREATION 
>   src/Makefile.am 814468e3c5c750a6649b5eeb7c7f945f9e025c19 
> 
> Diff: https://reviews.apache.org/r/34581/diff/
> 
> 
> Testing
> -------
> 
> * make check
> * run mesos:
> 1) build (make)
> 2) checked that *oversubscription.pb.h* and *oversubscription.pb.cc* are in 
> the proper directories
> 3) run *mesos-slave*, *mesos-master* and checked if their behaviour is proper
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Reply via email to