> On May 24, 2017, 10:26 a.m., Jiang Yan Xu wrote: > > Hmm, is this also violating the convention then? > > > > https://github.com/apache/mesos/blob/d225d4d4122e773e2416ba0d0eee653da8ced352/include/mesos/authorizer/acls.proto#L344 > > > > What if later we use uncountable nouns? > > > > FWIW when the rest of the ACLs read pretty natural to me, either the entity > > is a list of values, or the concept itself is plural (e.g., flags). > > Jiang Yan Xu wrote: > I am not so sure about this "convention". > > Entity is either a list of values or ANY/NONE type. If it's a list of > values, for sure the field name should be indicative of this. If it's a > ANY/NONE, why does it have to be plural? > > Alexander Rojas wrote: > I discussed that one too, but the idea there is that there is only one > log, while you have multiple agents.
Alright. I guess it comes down to whether you say "authorzie this action for ANY/No(ne) agent" or "authorzie this action for ANY/No(ne) agent**s**"? I preferred the former but I guess I was wrong about the English grammar. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59453/#review175959 ----------------------------------------------------------- On May 24, 2017, 1:12 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59453/ > ----------------------------------------------------------- > > (Updated May 24, 2017, 1:12 a.m.) > > > Review request for mesos, Adam B, Greg Mann, Michael Park, and Neil Conway. > > > Repository: mesos > > > Description > ------- > > Renames the field `RegisterAgent.agent` to `RegisterAgent.agents` in > order to come make it consistent with other ACLs. > > > Diffs > ----- > > include/mesos/authorizer/acls.proto > ae0b1ea2e6417d186b1606542d75f3a20e0811db > src/authorizer/local/authorizer.cpp > 89aaf4b712d337d519445c922606789c334e5101 > src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 > src/tests/master_authorization_tests.cpp > e4233c19b1d9e3e2734259503d0daec4ce243667 > src/tests/script.cpp 791d331d6bdf178098b2ce0dfb185f9128632459 > > > Diff: https://reviews.apache.org/r/59453/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
