Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103432 --- Ship it! Master (888f9a3) is green with this patch. ./build-supp

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-21 Thread Maxim Khutornenko
> On Oct. 20, 2015, 4:49 p.m., Joshua Cohen wrote: > > Not a blocker, but do you think the arrows in the output are necessary? > > They feel like extra noise to me. Maybe replace them with tabs instead? > > Maxim Khutornenko wrote: > Certainly not married to the arrows. Would you find this

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/ --- (Updated Oct. 21, 2015, 4:47 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-21 Thread Joshua Cohen
> On Oct. 20, 2015, 4:49 p.m., Joshua Cohen wrote: > > Not a blocker, but do you think the arrows in the output are necessary? > > They feel like extra noise to me. Maybe replace them with tabs instead? > > Maxim Khutornenko wrote: > Certainly not married to the arrows. Would you find this

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-20 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103330 --- Ship it! Master (888f9a3) is green with this patch. ./build-supp

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-20 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/ --- (Updated Oct. 20, 2015, 11:34 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-20 Thread Maxim Khutornenko
> On Oct. 20, 2015, 4:49 p.m., Joshua Cohen wrote: > > Not a blocker, but do you think the arrows in the output are necessary? > > They feel like extra noise to me. Maybe replace them with tabs instead? Certainly not married to the arrows. Would you find this easier to read? ``` $ aurora job d

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-20 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103268 --- Ship it! Not a blocker, but do you think the arrows in the output

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-19 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103189 --- Ship it! Unfortunately i'm only slightly better than a rubber stam

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-19 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103136 --- Ship it! Master (888f9a3) is green with this patch. ./build-supp

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-19 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/ --- (Updated Oct. 19, 2015, 5:23 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-19 Thread Maxim Khutornenko
> On Oct. 17, 2015, 2:54 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/client/cli/jobs.py, line 298 > > > > > > How does this play with an N-1 version scheduler that doesn't support > > `get_job_update_

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review103021 --- src/main/python/apache/aurora/client/cli/jobs.py (line 281)

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review102957 --- Ship it! Master (888f9a3) is green with this patch. ./build-supp

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/ --- (Updated Oct. 16, 2015, 6:03 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Maxim Khutornenko
> On Oct. 16, 2015, 5:19 p.m., Joshua Cohen wrote: > > Without digging into the code, just based on the output in the testing done > > section, would it be possible to collapse the output into instance ranges? > > So instead of [100, 101, 102, ... 119], we just get [100-119]? > > Maxim Khutorn

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Bill Farner
> On Oct. 16, 2015, 10:19 a.m., Joshua Cohen wrote: > > Without digging into the code, just based on the output in the testing done > > section, would it be possible to collapse the output into instance ranges? > > So instead of [100, 101, 102, ... 119], we just get [100-119]? > > Maxim Khutor

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Maxim Khutornenko
> On Oct. 16, 2015, 5:19 p.m., Joshua Cohen wrote: > > Without digging into the code, just based on the output in the testing done > > section, would it be possible to collapse the output into instance ranges? > > So instead of [100, 101, 102, ... 119], we just get [100-119]? Using ranges was

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review102936 --- Quick feedback on the output - printing ranges would be more useful

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review102934 --- Without digging into the code, just based on the output in the test

Re: Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/#review102844 --- Ship it! Master (888f9a3) is green with this patch. ./build-supp

Review Request 39366: Adding job update diff details into "aurora job diff" command.

2015-10-15 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39366/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1516