> On Sept. 2, 2014, 7: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. > > Mark Chu-Carroll wrote: > In this case, I disagree pretty strongly - but if you must have the > separate method, at least make the argument names match.
Feels like your disagreement is stronger than my preference in this case :). Changed. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 ----------------------------------------------------------- On Aug. 29, 2014, 10: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, 10: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 > >
