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

Revision history for this message
Gary Poster (gary) wrote :

On 04/11/2013 07:16 PM, <email address hidden> wrote:
>
> https://codereview.appspot.com/8680043/diff/1/app/views/charm.js#newcode100
>> app/views/charm.js:100: this.fire('navigateTo', {url: '/:gui:/'});
>> In cases like this I previously recommended that we use nsRouter.url
> to create
>> the URL. That no longer works in this case as '/' becomes implicit in
> the
>> namespace.
>
> For this case, right. I could use the flag I added, but I currently
> think that the string is fine for a root path.

Eh. I'm submitting this and I still don't know. Maybe what I did
first, post-review--"combine" excluded root paths by default and "url"
included them by default--was right. But that feels wrong too. Ugh.

If we determine that "url"'s utility has really been reduced by my
changes, then I would like to look at this again.

Thank you.

Gary

« Back to merge proposal