Skip to content
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

Remove Deprecated Fast Router #191

Merged
merged 3 commits into from
Feb 11, 2019
Merged

Conversation

marwan-at-work
Copy link
Contributor

Fixes #190 ✌️

@coveralls
Copy link

coveralls commented Feb 9, 2019

Coverage Status

Coverage increased (+0.3%) to 46.728% when pulling 02f5ef2 on marwan-at-work:rmf into 9eaa369 on NYTimes:master.

Copy link
Contributor

@jprobinson jprobinson left a comment

Choose a reason for hiding this comment

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

Thanks, Marwan! Should we also remove it from the mod file?

@fsouza
Copy link
Contributor

fsouza commented Feb 9, 2019

Tidy up

@marwan-at-work
Copy link
Contributor Author

@fsouza @jprobinson "go mod tidy" does not change anything, it's probably because server/kit still uses httprouter

@fsouza
Copy link
Contributor

fsouza commented Feb 11, 2019

:cries:

@jprobinson
Copy link
Contributor

Dang, I thought I had removed it from there but it was just marvin.

If you don't mind, let's rid ourselves of that package for good and remove it from the kit server as well?

@@ -28,7 +28,6 @@ require (
github.com/gorilla/mux v1.7.0
github.com/gorilla/websocket v1.4.0
github.com/grpc-ecosystem/go-grpc-middleware v1.0.0
github.com/julienschmidt/httprouter v1.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 yay!

Copy link
Contributor

@jprobinson jprobinson left a comment

Choose a reason for hiding this comment

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

🌯 !!! thanks @marwan-at-work!

@jprobinson jprobinson merged commit dc1269d into nytimes:master Feb 11, 2019
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.

None yet

4 participants