Skip to content

Remove gorilla mux #217

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove gorilla mux #217

wants to merge 1 commit into from

Conversation

Varsius
Copy link
Contributor

@Varsius Varsius commented Apr 1, 2025

No description provided.

@coveralls
Copy link

coveralls commented Apr 1, 2025

Coverage Status

coverage: 31.27% (+0.04%) from 31.23%
when pulling fb1740a on remove-gorilla-mux
into ba82c45 on master.

@Varsius Varsius force-pushed the remove-gorilla-mux branch from 3ede508 to fb1740a Compare April 3, 2025 09:47
@Varsius Varsius marked this pull request as ready for review April 3, 2025 09:50
Copy link
Contributor

@majewsky majewsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing gorilla/mux support all at once is too drastic of a change in my opinion. This would require immediately touching at least a dozen repos to update the API implementations. We should support a gradual move here.

I was initially considering something along the lines of

type API[Router interface { *mux.Router | *http.ServeMux }] interface {
  AddTo(r Router)
}

func Compose[Router interface { *mux.Router | *http.ServeMux }](apis ...API[Router]) http.Handler {
   ...
}

So then, each application can choose at compile time whether to use one or the other type. But this does not work for the API instances defined within this package (e.g. HealthCheckAPI) which would want to satisfy both interfaces. So we will definitely need to use separate names, and a new implementation should be added like:

type APIForServeMux interface {
  AddToServeMux(m *http.ServeMux)
}

func ComposeUsingServeMux(apis ...HttpAPI) http.Handler {
   ...
}

My main issue right now is that the type names are extremely awkward. I have not come up with good names here.

In fact, this is where I got stuck thinking about the problem a while ago, and why I didn't bother coming up with a PR myself yet.

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.

3 participants