-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RETARGET] supports subdomain #358
Conversation
@@ -91,7 +92,15 @@ type Router struct { | |||
var notFound = &RouteMatch{Action: "404"} | |||
|
|||
func (router *Router) Route(req *http.Request) *RouteMatch { | |||
leaf, expansions := router.Tree.Find(treePath(req.Method, req.URL.Path)) | |||
// by xiaoao[github.com/xiaoao] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not be tagging yourself in the code. It is in the git blame anyway.
ok,thank you |
@xiaoao you should write some unit tests ti ensure different subdomains are properly recognized (eg: some.domain.co.tld and other.long.sub.domains.com as well as a default for domain.com perhaps) |
@xiaoao have you been able to test that these changes work as expected? |
@@ -15,6 +15,7 @@ import ( | |||
|
|||
type Route struct { | |||
Method string // e.g. GET | |||
Subdomain string // e.g. subdomain:blog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this being used once it's set?
@xiaoao how would you define your routes with subdomains? |
See also: #536 |
Closing this one in favor of hosts |
I think this isn't a graceful way to support subdomain, and who will be nice enough to give me a better way
thx in advance