Refactor routes to use URL params instead of query params
Review Request #7006 — Created March 4, 2015 and submitted
Previously, routes were of the following format: /file?repo=:repo&id=:id. This isn't very clean and it's not obvious to the reader what it is doing. The design also isn't very RESTful. So the URLs have been refactored to look more closely to the following format: /repos/:repo/file/:id.
The new URLs are as followed:
Getting file blob, checking file existance:
/repos/:repo/file/:idGetting a file by commit and file path:
/repos/:repo/commits/:commit/path/:pathGetting the path of a repository:
/repos/:repo/pathGetting all branches:
/repos/:repo/branchesGetting all commits
/repos/:repo/branches/:branch/commits
OR /repos/:repo/branches/:branch/commits?start=:startGetting a commit (includes the diff):
/repos/:repo/commits/:idGetting the session:
/session
Verified that all new URLs still return the same contents as their old ones.
| Description | From | Last Updated |
|---|---|---|
|
I think it might be more readable to define this as a table and then add routes in a loop: … |
 david
|
-
Jessica,
It looks like this diff has a bunch of changes in it that aren't related to the URLs (and should maybe be in /r/6972/ instead). Can you clean it up?
JY
Change Summary:
Cleaning up as David suggested. P.S. I separated getFile and getFileByCommit into two separate functions in this rr, because of how the new routing works.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+98 -47) |
-
Tool: PEP8 Style Checker Ignored Files: README.md routes.go Tool: Pyflakes Ignored Files: README.md routes.go
-
Tool: PEP8 Style Checker Ignored Files: README.md routes.go Tool: Pyflakes Ignored Files: README.md routes.go
-
Tool: Pyflakes Ignored Files: README.md routes.go Tool: PEP8 Style Checker Ignored Files: README.md routes.go
-
-
routes.go (Diff revision 4) I think it might be more readable to define this as a table and then add routes in a loop:
routes := map[string]func { "/repos/{repo}/file/{id}": BasicAuth(getFile), "/repos/{repo}/commits/{commit}/path/{path}": BasicAuth(getFileByCommit), "/repos/{repo}/path": BasicAuth(getPath), "/repos/{repo}/branches": BasicAuth(getBranches), "/repos/{repo}/branches/{branch}/commits": BasicAuth(getCommits), "/repos/{repo}/commits/{id}": BasicAuth(getCommit), "/session": getSession, } for route, handler := range routes { r.HandleFunc(route, handler) }
JY
Change Summary:
Moving routes into a table as David suggested.
Depends On: |
|
||
|---|---|---|---|
Diff: |
Revision 5 (+104 -47) |
Tool: Pyflakes Ignored Files: repository.go git_repository.go README.md routes.go Tool: PEP8 Style Checker Ignored Files: repository.go git_repository.go README.md routes.go