Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Drop support for Go < 1.7: remove gorilla/context #391

Merged
merged 3 commits into from
Sep 2, 2018

Conversation

fharding1
Copy link
Contributor

Remove gorilla/context for reasons outlined in issue #326.

@elithrar
Copy link
Contributor

elithrar commented Jul 9, 2018

You’ll need to:

  • update travis.yml to remove versions < 1.7
  • update documentation to make it clear that clearing the context only applies when using > Go 1.7

@fharding1
Copy link
Contributor Author

fharding1 commented Jul 9, 2018

Alright. Also, I'm not sure, but it seems like we can remove contextClear from context_native and the code block from mux.go 158:160. If we did that Router.KeepContext would be basically useless (I think), but removing it would be breaking. Should I just put a comment on it that says this has no effect?

@flibustenet
Copy link

Actually gorilla/context is required.
I think it will be more explicit to update go.mod to reflect this now with a require github.com/gorilla/context, then release the last mux that require it and then remove it in this PR.

@elithrar
Copy link
Contributor

@fharding1 Did you want to make the adjustment here as per @flibustenet's comment?

@fharding1
Copy link
Contributor Author

I'm not sure what he means, why is it required?

@elithrar
Copy link
Contributor

elithrar commented Aug 28, 2018 via email

@fharding1
Copy link
Contributor Author

I'm not super familiar with modules, but yeah, I don't think it's required. Wasn't planning on addressing that feedback unless @flibustenet can clarify why it's required.

@flibustenet
Copy link

I was just thinking of the historical side. We could see it was required and not after. But the changelog is ok also, nothing more...

@elithrar elithrar requested review from elithrar and kisielk August 31, 2018 17:23
Copy link
Contributor

@elithrar elithrar left a comment

Choose a reason for hiding this comment

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

Some minor build & filename adjustments required, otherwise a straightforward change!

@fharding1 fharding1 force-pushed the fsh/drop-gorilla-context branch from 3615587 to 59ccdf0 Compare August 31, 2018 18:27
Copy link
Contributor

@elithrar elithrar left a comment

Choose a reason for hiding this comment

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

Thanks @fharding1

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants