Skip to content

Fix README#250

Merged
excpt merged 1 commit intojwt:masterfrom
rono23:fix-readme
Mar 13, 2018
Merged

Fix README#250
excpt merged 1 commit intojwt:masterfrom
rono23:fix-readme

Conversation

@rono23
Copy link
Copy Markdown
Contributor

@rono23 rono23 commented Dec 27, 2017

I think sub should be a symbol, not string.

@sourcelevel-bot
Copy link
Copy Markdown

Hello, @rono23! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@sourcelevel-bot
Copy link
Copy Markdown

Ebert has finished reviewing this Pull Request and has found:

  • 1 fixed issue! 🎉

You can see more details about this review at https://ebertapp.io/github/jwt/ruby-jwt/pulls/250.

Copy link
Copy Markdown

@Spone Spone left a comment

Choose a reason for hiding this comment

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

If it's a symbol, the correct syntax is { :sub => sub, ... not { sub: => sub, ...

@excpt excpt self-requested a review January 9, 2018 23:02
@excpt
Copy link
Copy Markdown
Member

excpt commented Jan 9, 2018

@rono23 Thanks for the PR.
@Spone Thanks for the review.

I know the documentation is not up to date.

I recommend using the preferred hash syntax.

{key: value}

@Spone
Copy link
Copy Markdown

Spone commented Jan 9, 2018

Yup, even better 👌

@rono23
Copy link
Copy Markdown
Contributor Author

rono23 commented Jan 10, 2018

@Spone @excpt Thank you for the review! I updated my commit. Please review again 🙇

Copy link
Copy Markdown
Member

@excpt excpt left a comment

Choose a reason for hiding this comment

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

Thank you very much! 👍

@excpt excpt merged commit c6643e3 into jwt:master Mar 13, 2018
@rono23 rono23 deleted the fix-readme branch March 13, 2018 12:33
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.

3 participants