Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187455 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 5,

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187437 --- ui/src/main/js/components/__tests__/InstanceHistoryItem-test.js

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187435 --- Ship it! LGTM, except missing some test cases for showing

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187221 --- Ship it! Master (0f1e684) is green with this patch.

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 5, 2017, 12:28 a.m., Kai Huang wrote: > > ui/src/main/js/components/StateMachine.js > > Lines 37 (patched) > > > > > > Add a key property here to suppress the warnings in the browser console? Done. -

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/ --- (Updated Oct. 5, 2017, 11:50 p.m.) Review request for Aurora, Kai Huang and

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187150 --- ui/src/main/js/components/StateMachine.js Lines 37 (patched)

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 11:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:01 p.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 21 (original), 22 (patched) > > > > > > I'm not sure why I cannot access the job page and the instance page. > > Visiting this

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI.

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:37 p.m., Stephan Erb wrote: > > ui/src/main/js/utils/Task.js > > Lines 40-41 (patched) > > > > > > Is the array index safe here? If we need the guard for > > `task.taskEvents.length` in

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187132 --- Regarding the question nodejs vs js thrift: nodejs supports both

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187128 --- Looks great! :) When playing around with the UI on the vagrant

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 9:01 p.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 21 (original), 22 (patched) > > > > > > I'm not sure why I cannot access the job page and the instance page. > > Visiting this

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187129 --- ui/src/main/js/index.js Line 21 (original), 22 (patched)

Re: Review Request 62720: Implement Instance pages in React

> On Oct. 4, 2017, 5:50 p.m., Reza Motamedi wrote: > > ui/.eslintrc > > Lines 15 (patched) > > > > > > For my education, does this set `ACTIVE_STATES`, `Thrift`, ... up for > > you to later assign values to them?

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187110 --- ui/.eslintrc Lines 15 (patched)

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187052 --- Ship it! Master (4e7cdc4) is green with this patch.

Re: Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/ --- (Updated Oct. 4, 2017, 5:43 a.m.) Review request for Aurora, Kai Huang and

Review Request 62720: Implement Instance pages in React

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.