> 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? > > Marco Massenzio wrote: > 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 :)
https://msdn.microsoft.com/en-us/library/141e06ef(v=vs.71).aspx. I know it's MS but google style guide has no rule for 2 chars long acronyms/abbreviations and it's very common. "would be a bit of a challenge, don't you think? :)" - totally agree (: I.D... Wow i wasn't aware of that! =D So do you agree that ID is an *acronym* not *abbreviations*? If that's true then this also a valid case. (: - Bartek ----------------------------------------------------------- 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 > >
