Return-Path: X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9B77710B29 for ; Fri, 4 Apr 2014 23:50:55 +0000 (UTC) Received: (qmail 54282 invoked by uid 500); 4 Apr 2014 23:50:53 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 54243 invoked by uid 500); 4 Apr 2014 23:50:53 -0000 Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.incubator.apache.org Delivered-To: mailing list reviews@aurora.incubator.apache.org Received: (qmail 54234 invoked by uid 99); 4 Apr 2014 23:50:53 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 04 Apr 2014 23:50:53 +0000 X-ASF-Spam-Status: No, hits=-1998.3 required=5.0 tests=ALL_TRUSTED,HTML_MESSAGE,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Fri, 04 Apr 2014 23:50:49 +0000 Received: (qmail 54064 invoked by uid 99); 4 Apr 2014 23:50:25 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 04 Apr 2014 23:50:25 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id C306F1D5D70; Fri, 4 Apr 2014 23:50:21 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6863246509503911337==" MIME-Version: 1.0 Subject: Re: Review Request 19833: Migrated Job page to angular JS From: "Suman Karumuri" To: "Bill Farner" , "Kevin Sweeney" Cc: "Aurora" , "Suman Karumuri" Date: Fri, 04 Apr 2014 23:50:21 -0000 Message-ID: <20140404235021.17825.92159@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Suman Karumuri" X-ReviewGroup: Aurora X-ReviewRequest-URL: https://reviews.apache.org/r/19833/ X-Sender: "Suman Karumuri" References: <20140403184215.16142.19148@reviews.apache.org> In-Reply-To: <20140403184215.16142.19148@reviews.apache.org> Reply-To: "Suman Karumuri" X-ReviewRequest-Repository: aurora X-Virus-Checked: Checked by ClamAV on apache.org --===============6863246509503911337== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit > On April 3, 2014, 6:42 p.m., Bill Farner wrote: > > Tried this out locally, overall it looks great! > > > > Some things i'd like to see addressed: > > 1. The 'gear' links result in "An error occurred when querying the server". Looks like your query swapped the job name with task ID. > > To be more discrete, and not present the unlabeled column, perhaps the instance ID could be a link (as opposed to the gear). > > 2. Column widths are inconsistent with content (instance ID column is nearly 1/2 the table width) > > 3. Clicking the + to expand status history for a task causes columns to shift, probably fixed by (2) > > 4. The 'Hide Summary' button should be vertically separated from the 'Active tasks' heading, rather than directly to the right > > (similar issue is present for 'Show Resource Consumption' on the role page) > > 5. The 'Last Active' column is confusing, can you rework this to match the existing columns? > > > > Bill Farner wrote: > Also, the job page has a Search input, but AFAICT it doesn't do anything. I vote to remove it. Removed the gear. Added π. Fixed the column widths and status expansion. Vertically aligned the summary buttons on job and role pages. Removed last active column. Disabled search on job page. - Suman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review39449 ----------------------------------------------------------- On April 2, 2014, 11:26 p.m., Suman Karumuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19833/ > ----------------------------------------------------------- > > (Updated April 2, 2014, 11:26 p.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-281 > https://issues.apache.org/jira/browse/AURORA-281 > > > Repository: aurora > > > Description > ------- > > *** Please review Diff 2. Diff 1 is https://reviews.apache.org/r/19565/. Added it here so only new code can be reviewed. *** > > > Migrated job page to AngularJS. > Removed old scheduler job page. > Removed js related to the old scheduler page. Removed data tables. > Added moment.js library for date time manipulation. > Refactored code a bit for reusability. > Stats link should still be added to the new jobs page. > > Since the role/env code is not committed, this diff contains that code as well. > Code changes start from page 4 in diff between version 1 and 2. > > > Diffs > ----- > > build.gradle 109c193da3324bd5534b409bfabb6aeb0adda7b1 > src/main/java/org/apache/aurora/scheduler/app/AppModule.java eeafc784e915137cacd5f64df1252ccbaf6c0f6c > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java ec56c649116c03ef148bac916bd6691a94685bc3 > src/main/java/org/apache/aurora/scheduler/http/DisplayUtils.java 19df7889f15b4cf44e386d8ce0626cc94fdcdfba > src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 2ccc6f367b9715a0abb3e0673069289ae4860087 > src/main/java/org/apache/aurora/scheduler/http/ServletModule.java e3ff2571d95effcf72b2047cc5840d56143a180c > src/main/java/org/apache/aurora/scheduler/http/UIRedirectFilter.java b35aad8db006133ba70692e43db1f083d4950914 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/css/jquery.dataTables.css ee6da23c3948a87b0d4df73e82e5c4e2ab2bc803 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/back_disabled.png 881de7976ff98955e2a5487dca66e618a0655f3d > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/back_enabled.png c608682b04a6d9b8002602450c8ef7e80ebba099 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/back_enabled_hover.png d300f1064b3beac1d7d5274e294494d3143e53a2 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/forward_disabled.png 6a6ded7de821619aedc71d1738c0b73463a4452e > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/forward_enabled.png a4e6b5384b8454ee7f44a8f7c75b0321b7eeb9b1 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/forward_enabled_hover.png fc46c5ebf0524b72a509fe2d7c1bc74995cb8a9d > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/sort_asc.png a88d7975fe9017e4e5f2289a94bd1ed66a5f59dc > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/sort_asc_disabled.png 4e144cf0b1f786a9248a2998311e8109998d8a2d > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/sort_both.png 18670406bc01ab2721781822dd6478917745ff54 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/sort_desc.png def071ed5afd264a036f6d9e75856366fd6ad153 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/images/sort_desc_disabled.png 7824973cc60fc1841b16f2cb39323cefcdc3f942 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/js/dataTables.bootstrap.js 420e507086f7a2af08f68787f67208546745260d > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/js/dataTables.htmlNumberType.js 885ad347f3076dcf75558ef44806dc25a8171884 > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/js/dataTables.localstorage.js 3c0b39aefbd5a82adee49f2ac7539b6974576b2f > src/main/resources/org/apache/aurora/scheduler/http/assets/datatables/js/jquery.dataTables.min.js 02694a4a56eb247baa2398d971927dfbd1ac3e60 > src/main/resources/org/apache/aurora/scheduler/http/assets/dictionary.js 92045c41706ee9315f3522c3b5bbff7f6e7d2c64 > src/main/resources/org/apache/aurora/scheduler/http/assets/util.js 5dd0d17849c7116d70925275b9c6c0715b441f14 > src/main/resources/org/apache/aurora/scheduler/http/schedulerzjob.st 28b56671b2e825912a6427e609c2bbe1e7758e26 > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css ade850ce624964693e9bd55946464983c4b9f8c2 > src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 36225d1e5147e30ba2cb4ddda96dec9f0f2f1dce > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js db6ea99aeb749fd8674613e3620dc3012872e13c > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 7cd534479dd2f17ffd46248ce9af1f8fe89beb97 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js d2b2017a0efc70d425fd6c89ad6caaf46cb8ded5 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8681bbe840b6285b15dd6766016258c2c8115632 > src/main/resources/org/apache/aurora/scheduler/http/ui/schedulingDetail.html PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/taskLink.html PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/taskSandbox.html PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/taskStatus.html PRE-CREATION > src/main/thrift/org/apache/aurora/gen/api.thrift c0618e4edebd6f282698abfd9bdc3c36fff16920 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a4e9464f7d5d3f5a640b62557c3e29f2f1566985 > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java fae2de11235dd059718e1023fdcfb0e8fc4deadd > src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java dd991fb90889c3f221e537a78283eb4a31e5f0dd > src/test/resources/org/apache/aurora/gen/api.thrift.md5 05c6e8a1e2407f2822b3844c555d8995f1cd1d49 > > Diff: https://reviews.apache.org/r/19833/diff/ > > > Testing > ------- > > ./gradlew clean build run on local laptop. > > > Thanks, > > Suman Karumuri > > --===============6863246509503911337==--