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

Added _ = to indicate there is a return from Save #197

Merged
merged 3 commits into from Sep 17, 2019

Conversation

adamjack
Copy link
Contributor

This is because I copied the sample and failed to notice there was a return, which for me was failing, and lost a bunch of time to troubleshooting. (Note: I actually copied the samples from here - https://www.gorillatoolkit.org/pkg/sessions - but don't know how to update that page.)

Fixes #

Summary of Changes

  1. Added _ = 'cos even though the error is ignored this shows one exists

PS: Make sure your PR includes/updates tests! If you need help with this part, just ask!

This is because I copied the sample and failed to notice there was a return, which for me was failing, and lost a bunch of time to troubleshooting.
Indicated the need for error handling so developer who copy samples get the indication that error handling is required. ( I was learning/tinkering and copied a sample without error handling, failed to notice that Save returns an error in the documentation, and it took me a lot of time/troubleshooting to track down the problem.)
@adamjack
Copy link
Contributor Author

Used _ = in the README.md (so as not to clutter / obscure the message) and "error =" & "if error != nil" in the docs.go to be more explicit.

@stale
Copy link

stale bot commented Sep 8, 2019

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

@stale stale bot added the stale label Sep 8, 2019
@elithrar elithrar self-assigned this Sep 8, 2019
@stale stale bot removed the stale label Sep 8, 2019
@elithrar elithrar self-requested a review September 8, 2019 23:01
README.md Outdated
@@ -41,7 +41,7 @@ Let's start with an example that shows the sessions API in a nutshell:
session.Values["foo"] = "bar"
session.Values[42] = 43
// Save it before we write to the response/return from the handler.
session.Save(r, w)
_ = session.Save(r, w)
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle the error - e.g. if the session save fails, log it.

@adamjack
Copy link
Contributor Author

adamjack commented Sep 9, 2019

@elithrar Thanks for the feedback/change request. I was attempting to be consistent with the use of "_" in error handling a few lines above, but I've made a change to match your change request. I didn't log so much as return the HTTP internal error, as I found in other places in the Gorilla Sessions documentation. If you want just logging instead I can do that.

@elithrar
Copy link
Contributor

Thanks!

@elithrar elithrar merged commit daaabe7 into gorilla:master Sep 17, 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

2 participants