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

Sinatra headers only accept string hash keys #12

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

koffeinfrei
Copy link
Contributor

The current example handler doesn't work.

The sinatra header hash needs to consists of string keys, not symbol keys. Currently we get:

ERROR NoMethodError: undefined method `gsub' for :"content-type":Symbol

@derek derek bot added the no-dco label Oct 20, 2019
@derek
Copy link

derek bot commented Oct 20, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@derek derek bot removed the no-dco label Oct 20, 2019
Signed-off-by: Alexis Reigel <mail@koffeinfrei.org>
Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

LGTM

For future commit messages, please could you see Chris Beam's advice?

In particular, the commit message lacks the "why" this change was made.

https://chris.beams.io/posts/git-commit/

@alexellis alexellis merged commit c863232 into openfaas:master Oct 21, 2019
@alexellis
Copy link
Member

Thanks for fixing this.

@koffeinfrei
Copy link
Contributor Author

In particular, the commit message lacks the "why" this change was made.

@alexellis sinatra headers only accept string hash keys is the "why". Do you mean the commit message lacks the "what"?

@koffeinfrei koffeinfrei deleted the fix/header-keys-as-strings branch October 21, 2019 08:27
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