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

Chore/extract reused hash key #34

Merged
merged 2 commits into from
Jul 14, 2014

Conversation

rud
Copy link
Contributor

@rud rud commented Apr 28, 2014

Small cleanups, hope they are considered helpful.

Feedback most welcome.

@Houdini
Copy link
Owner

Houdini commented May 31, 2014

Hello, there is a problem in rails 4.1.1 if you use symbols for cookies keys, so we change it to strings.
In this circumstances your commit become much more relevant.

Could you please change following:

  1. NEED_AUTHENTICATION should define string not symbol (please check last commits)
  2. Please add small comment which explains that you use this as cookies key. Or maybe it would be better to rename NEED_AUTHENTICTION to more descriptive name, like COOKIE_KEY

Thanks

@rud
Copy link
Contributor Author

rud commented Jul 14, 2014

Great feedback, thank you.

I've force-pushed an update to my repository fixing the cookie-key to be a string instead of a hash. Unfortunately the github UI has not been correctly updated to reflect this. If you try the branch locally it will merge without any problems.

@Houdini Houdini merged commit 20703c0 into Houdini:master Jul 14, 2014
@Houdini
Copy link
Owner

Houdini commented Jul 14, 2014

Thanks

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