> On Oct. 20, 2014, 11:11 p.m., Joshua Cohen wrote: > > It's not entirely clear from the screenshot that "LOCAL" is a reference to > > the timezone. Maybe it's the fact that it's capitalized, but it feels more > > related to the task state than the time (I'm envisioning questions like > > "what does LOCAL - PENDING mean?"). Am I being crazy? Would it make sense > > to change it to something like "(local time)", or even better, instead of > > hardcoding "LOCAL" maybe just display their actual time zone identifier > > (or, since that's a minor hassle from JS, maybe just the offset)? > > David McLaughlin wrote: > Hmmm, I can see your point but I found the underline/highlight on the > status made it very clear they were separate entities. Is it too subtle? We > also have a request to show UTC so it might be time to rethink the display of > task events. > > (FWIW I would much rather show timezone, but it is deprecated in > moment.js for the usual reasons and I don't think offset is a good user > experience.) > > Joshua Cohen wrote: > Is http://momentjs.com/timezone/ what's deprecated? Or are we just wary > of introducing another dependency? > > David McLaughlin wrote: > That is a library to replace the deprecation I was talking about (you can > use 'z' in the formatting string, but it hard fails in certain time zones). > But yeah I'd prefer to avoid adding a new dependency to solve this, at least > until we do a better job with JS dependency management. > > David McLaughlin wrote: > Alternative proposal: wrap toLocalTime in a try catch and only show local > if the moment.js call using 'z' fails? > > Joshua Cohen wrote: > Sounds reasonable. > > Is there a ticket tracking the larger problems w/ JS depdency management? > > David McLaughlin wrote: > There is sort of a catch-all of a bunch of issues here: > https://issues.apache.org/jira/browse/AURORA-451 > > Kevin Sweeney wrote: > What's the problem with JS dependency management? Does [this > process](https://github.com/apache/incubator-aurora/blob/master/docs/developing-aurora-scheduler.md#developing-aurora-ui) > not work? > > David McLaughlin wrote: > Yes it works, but it means tracking a bunch of 3rdparty source code in > our repo. These should be pulled in and installed (and unified/mindified into > a single payload) as part of a build process.
That ticket captures the work to get a test framework set up, which we certainly need, but I'm not clear on how it relates to adding new dependencies? It might change how dependencies are managed, but it might not, and given that that work hasn't been prioritized for ~5 months, I'm not sure it should be a blocker to introducing new JS dependencies. - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26956/#review57459 ----------------------------------------------------------- On Oct. 20, 2014, 10:55 p.m., David McLaughlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26956/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2014, 10:55 p.m.) > > > Review request for Aurora, Joshua Cohen and Kevin Sweeney. > > > Bugs: AURORA-873 > https://issues.apache.org/jira/browse/AURORA-873 > > > Repository: aurora > > > Description > ------- > > Fixes the AM/PM issue by using 24 hour clock format that was already defined > in filters.js. > > > Diffs > ----- > > src/main/resources/scheduler/assets/js/controllers.js > f8e2eb7b896843bb19ad3ea5108532209603c73c > src/main/resources/scheduler/assets/taskStatus.html > ae32866ff241e18d04a82e8a7f909cb692657188 > > Diff: https://reviews.apache.org/r/26956/diff/ > > > Testing > ------- > > See attached screenshot. > > > File Attachments > ---------------- > > screenshot > > https://reviews.apache.org/media/uploaded/files/2014/10/20/d89be4fc-7fae-4746-841a-ccaf66e2439e__Screen_Shot_2014-10-20_at_3.55.15_PM.png > > > Thanks, > > David McLaughlin > >
