> On Sept. 2, 2014, 3:35 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 176 > > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line176> > > > > Nit - but why are you changing the parameter comment syntax? We don't > > use the double-dash anywhere else in the client. > > Maxim Khutornenko wrote: > This is the style everything under client/api seems to follow. Just > making it consistent.
Ick. So client/api doesn't follow the same pattern as a lot of other client code. Something for me to fix at some point. > On Sept. 2, 2014, 3:35 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 232 > > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232> > > > > I think this would be clearer inlined. Right now, it's pretty much an > > alternate name for the JobUpdateQuery constructor, but with different > > parameter names. It makes the code harder to follow, not easier. > > Maxim Khutornenko wrote: > The reason for this is mostly consistency (see build_query above) but I > personally prefer this approach as it decouples query building from the > thrift API call. In this case, I disagree pretty strongly - but if you must have the separate method, at least make the argument names match. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 ----------------------------------------------------------- On Aug. 29, 2014, 6:28 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25204/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2014, 6:28 p.m.) > > > Review request for Aurora and Mark Chu-Carroll. > > > Bugs: AURORA-615 > https://issues.apache.org/jira/browse/AURORA-615 > > > Repository: aurora > > > Description > ------- > > Adding "get" job update client APIs. > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/__init__.py > 90462bf5920786ea1316c75a99c8382cf8c803a1 > src/test/python/apache/aurora/client/api/test_api.py > b47b6db31842fffba797c7f616b5f4deb8d04a86 > > Diff: https://reviews.apache.org/r/25204/diff/ > > > Testing > ------- > > ./pants src/test/python/apache/aurora/client/api:api -s > > > Thanks, > > Maxim Khutornenko > >
