Conversation
example_router_test.go
Outdated
| @@ -0,0 +1,25 @@ | |||
| package mux_test | |||
There was a problem hiding this comment.
We should move this test case in the file: example_route_test.go
| return &c | ||
| } | ||
|
|
||
| func (r *Router) RegisterPattern(alias string, pattern string) *Router { |
There was a problem hiding this comment.
Can we add comments here?
| buildScheme string | ||
|
|
||
| buildVarsFunc BuildVarsFunc | ||
|
|
There was a problem hiding this comment.
Same, can we add comment here as well?
| host: "aaa.bbb.ccc", | ||
| path: "", | ||
| hostTemplate: `{v-1:version}.{v-2:version}.{v-3:version}`, | ||
| shouldMatch: true, |
There was a problem hiding this comment.
Can we add negative test case where shouldMatch is false because of mismatch.
mux_test.go
Outdated
| }, | ||
| { | ||
| title: "Path route with regexp alias patterns passed through router", | ||
| route: NewRouter().RegisterPattern("digits", "[0-9]+").Path("/{id:digits}"), |
There was a problem hiding this comment.
Both the test cases look similar to me.
- First one:
{
title: "Path route with regexp alias patterns",
route: new(Route).RegisterPattern("digits", "[0-9]+").Path("/{id:digits}"),
request: newRequest("GET", "http://localhost/1"),
vars: map[string]string{"id": "1"},
host: "",
path: "/1",
pathTemplate: `/{id:digits}`,
shouldMatch: true,
}- Second:
{
title: "Path route with regexp alias patterns passed through router",
route: NewRouter().RegisterPattern("digits", "[0-9]+").Path("/{id:digits}"),
request: newRequest("GET", "http://localhost/1"),
vars: map[string]string{"id": "1"},
host: "",
path: "/1",
pathTemplate: `/{id:digits}`,
shouldMatch: true,
}We should add at least one negative test case. What do you think?
There was a problem hiding this comment.
Yeah, seems like this tests was very similar, deleted second test and added negative test case
|
Thank you @HiveTraum for bringing this up again. I have added a couple of comments. I need some more time to review it completely. In the meantime, I Would love to hear from you. |
|
@amustaque97 Done 👍 |
amustaque97
left a comment
There was a problem hiding this comment.
Thank you for working on comments. Changes looks good to me.
|
@elithrar , I reviewed the PR and changes looks good to me and ready to ship 🚀 I would request you to please take a final look at this and if all good. Can we merge it? |
|
Yes, it's ready to merge |
✋Reopened #598 because i noticed that the project came to life
This feature introduces ability to register aliases for some often used regular expressions.
For example path as this:
/{category:[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}}/{product:[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}}can be transformed into this
/{category:uuid}/{product:uuid}You need just use register it
RegisterPatternonRoutestructRegisterPatternonRouterstruct