Skip to content

Added mass register funcs and example used as test. - #1

Open
sqp wants to merge 1 commit into
bosssauce:masterfrom
sqp:master
Open

Added mass register funcs and example used as test.#1
sqp wants to merge 1 commit into
bosssauce:masterfrom
sqp:master

Conversation

@sqp

@sqp sqp commented Aug 11, 2017

Copy link
Copy Markdown
  • The router type now just extends the mux.Router so we can use it directly (and get the returned object from HandleFunc to add more options).
  • RegisterGet and RegisterPost are used to add lists of routes (see example).
  • The example shows how to register handlers and 2 of its 3 paths are used in the test (need POST).

- The router type now just extends the mux.Router so we can use it directly (and get the returned object from HandleFunc to add more options).
- RegisterGet and RegisterPost are used to add lists of routes (see example).
- The example shows how to register handlers and 2 of its 3 paths are used in the test (need POST).
@nilslice

Copy link
Copy Markdown
Member

@sqp - this looks great, and I like how you designed the RegisterGet/Post methods to make a more straightforward API, but I have a couple comments. Also, I had initially decided not to embed the *mux.Router so that it would be easy to swap it out for another if that was something we wanted to do later, while still making it accessible outside the package. If you think it is definitely better off embedded, please share!

I think the ListHande, ToHandler, and ToHandle types might be unnecessary.. could we get away with just using a map[string]http.HandleFunc instead? Requiring the package user to learn this syntax:

frontend.RegisterGet(frontend.ListHandle{{
	URL:     "/",
	Handler: viewHomePage,
}, {
	URL:     "/other/{keyName}",
	Handler: viewOther,
}}.ToHandlers()...)

when it's possible to do something like:

frontend.RegisterGet(frontend.Route{
	"/":                viewHomePage,
	"/other/{keyName}": viewOther,
})

I think keeping the new syntax to a minimum helps a lot to improve usability... what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants