Code review comment for lp:~gary/juju-gui/bug1203583

On 2013/10/08 21:03:10, jeff.pihach wrote:
> LGTM QA OK with trivial

> https://codereview.appspot.com/14565044/diff/1/app/models/charm.js
> File app/models/charm.js (right):

https://codereview.appspot.com/14565044/diff/1/app/models/charm.js#newcode466
> app/models/charm.js:466: setter: function(value) {
> you can quote this instead of commenting it if you like.

https://codereview.appspot.com/14565044/diff/1/app/models/charm.js#newcode468
> app/models/charm.js:468: value.sort(function(a, b) {
> This all looks great but I'm curious what this sort method gets you
over the
> standard lexicographic sort. You may also want to add this comment to
the code
> so others know when looking at this.

Thank you for the review and comments, Jeff. I added the following
explanation to the code.

             // This sort has several properties that are different than
a
             // standard lexicographic sort.
             // * Filenames in the root are grouped together, rather than
             // sorted along with the names of directories.
             // * Sort ignores case, unless case is the only way to
             // distinguish between values, in which case it is honored
             // per-directory. For example, this is sorted: "a", "b/c",
             // "b/d", "B/b", "B/c", "e/f". Notice that "b" directory
values
             // and "B" directory values are grouped together, where
they
             // would not necessarily be in a simpler case-insensitive
             // lexicographic sort.
             // If you rip this out for something different and simpler,
that's
             // fine; just don't tell me about it. :-) This seemed like
a good
             // approach at the time.

https://codereview.appspot.com/14565044/