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

Fix parsing of a JSON event #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Sod-Almighty
Copy link

Hint: JSON contains colons

Hint: JSON contains colons.
Fix parsing of a JSON event
@mistersourcerer
Copy link
Owner

mistersourcerer commented May 25, 2018

Thank you very much for your interest on the project @Sod-Almighty. Even more for taking the time to try and make it better.

I would like to ask you two things if possible:

  • Could you write a spec that exemplify the problem? If I understood right this is to avoid errors if the json field is something like {omg: }, right?
  • Can you squash your commits so the PR is made of only one commit? I'm in favor of multiple commits for more complex changes, but this one is very to the point. wdyt?

Copy link
Owner

@mistersourcerer mistersourcerer left a comment

Choose a reason for hiding this comment

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

Great stuff indeed! 🎉
Thanks for that.

Would you be so kind to write a spec that cover this problem so we avoid to have it again in the future?

@Sod-Almighty
Copy link
Author

Hi. I'm afraid I don't know how to do any of these things. My understanding of Git and such is very limited.

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