> On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 606
> > <https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606>
> >
> >     You can just do BaseJobController($scope, ...) now? Here and in other 
> > invocations.
> 
> Joshua Cohen wrote:
>     This is pretty standard mechanism for inheritance in JS. We could remove 
> the .call invocation, but then if something changed in the future that *did* 
> bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it 
> as is.
> 
> David McLaughlin wrote:
>     So maybe I don't understand why you're using classical inheritance here 
> then, over a vanilla function?
> 
> Joshua Cohen wrote:
>     I guess it just made more sense conceptually, but maybe that's because 
> I've been spending way too much time writing java lately ;). Do you think a 
> vanilla function is easier to understand in this context?

Yeah I mean if we keep it this way, it seems to be advocating an approach where 
any simple function needs to be explicit about what 'this' is when invoked, 
just in case it's ever used? Does that make sense?


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37365/#review95427
-----------------------------------------------------------


On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37365/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 2:44 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add a new UI page to show all tasks (active and completed) for a specific 
> instance id.
> 
> N.B.: changes to controllers.js look significant, but it's 90% refactoring so 
> that JobController and JobInstanceController can reuse common code.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/base.py 
> dee4c28a26828912bec073c7e5213a03c9703eee 
>   src/main/resources/scheduler/assets/breadcrumb.html 
> 13147935c3c28680d5decd1e03e40d9cf04aaed3 
>   src/main/resources/scheduler/assets/css/app.css 
> a4735efe58a09a46f2b95ebe98f219cb9e7ee27d 
>   src/main/resources/scheduler/assets/instance.html PRE-CREATION 
>   src/main/resources/scheduler/assets/job.html 
> 942635a61132bb9a38a6faf3f8fe046e6bcabd95 
>   src/main/resources/scheduler/assets/js/app.js 
> b66409f628046c67b62c92a75cbeed20c09b4ec1 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c 
>   src/main/resources/scheduler/assets/js/services.js 
> a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d 
>   src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 21bea702a6e6fe68652e2195c5d24b56b6477738 
>   src/test/python/apache/aurora/client/test_base.py 
> 1a560088279ac945cce14d02454e50b8483771e4 
> 
> Diff: https://reviews.apache.org/r/37365/diff/
> 
> 
> Testing
> -------
> 
> - ./gradlew jsHint
> - ./pants test src/test/python/apache/aurora/client:base
> - Verified update page url in client was correct in Vagrant.
> - See attached screenshot for an example of what the instance page looks like.
> 
> 
> File Attachments
> ----------------
> 
> Example of instance page in action.
>   
> https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Reply via email to