Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(822)

Issue 9830046: Better Hash/QS support in router

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by bcsaller
Modified:
11 years, 10 months ago
Reviewers:
rharding, j.c.sackett, mp+168351
Visibility:
Public.

Description

Better Hash/QS support in router `parse`, `url` and `combine` deal more gracefully with hash and qs url options. In the case of combine the incoming url replaces any existing values in the hash and qs portions of the URL as might be expected. (This is rather than some artificial merge policy for example). https://code.launchpad.net/~bcsaller/juju-gui/routing-hash-query/+merge/168351 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Better Hash/QS support in router #

Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -16 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/assets/javascripts/ns-routing-app-extension.js View 1 7 chunks +71 lines, -16 lines 0 comments Download
M test/test_routing.js View 2 chunks +18 lines, -0 lines 0 comments Download

Messages

Total messages: 4
bcsaller
Please take a look.
11 years, 10 months ago (2013-06-10 06:03:30 UTC) #1
rharding
LGTM with a concern about the hash parsing to test/check on please. https://codereview.appspot.com/9830046/diff/1/app/assets/javascripts/ns-routing-app-extension.js File app/assets/javascripts/ns-routing-app-extension.js ...
11 years, 10 months ago (2013-06-10 11:50:50 UTC) #2
j.c.sackett
LGTM, with one question about using a regex over split, largely for my own edification. ...
11 years, 10 months ago (2013-06-10 13:27:06 UTC) #3
bcsaller
11 years, 10 months ago (2013-06-10 22:11:41 UTC) #4
Please take a look.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b