aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David McLaughlin" <da...@dmclaughlin.com>
Subject Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
Date Fri, 14 Aug 2015 16:47:59 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message