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 sendJSON method #4

Merged
merged 3 commits into from
Feb 1, 2018
Merged

Conversation

bl4ckm0r3
Copy link
Contributor

I have found myself writing a lot of JSON.stringify, so i made an utility method that will do it for you!

@bl4ckm0r3 bl4ckm0r3 force-pushed the feature/add-sendJSON branch from 3f43bff to abda556 Compare January 31, 2018 13:05
Copy link
Owner

@lukeed lukeed left a comment

Choose a reason for hiding this comment

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

Hey, cool! 😃 Not sure why I didn't think to include this, I do it all the time too.

I'll make the changes, but how do you feel about calling it json instead of sendJSON?

@bl4ckm0r3
Copy link
Contributor Author

No worries, that's the beauty of opensource!
Tbh i think JSON is kind of all caps (acronym).
I did a sendJson first but then changed for standard.
I think sendjson doesn't read as well as sendJSON or sendJson.
What do you think?
I am open to suggestions :)

@bl4ckm0r3
Copy link
Contributor Author

I have just realised you meant just json instead of sendJSON.
I think clarity is better and sendJSON describes better what is going on.
But it's your library :)

@lukeed
Copy link
Owner

lukeed commented Jan 31, 2018

No problem 😆 I'm leaning towards json() because of bytes (lol) and it looks more like the Express response helper (res.json({...})), and people may be used to that.

Lemme ask around real quick for opinions and I'll get back shortly~

@SinisterBlade
Copy link

FWIW, I think just json() is clear enough. (And it looks prettier too! 😊)

@lukeed
Copy link
Owner

lukeed commented Feb 1, 2018

Me too 😄 That was also the consensus I got today when I asked others. 🙌

Calls to `ws.send` don't return anything, so there's no need to return anything either~
@lukeed lukeed merged commit f3f0bbd into lukeed:master Feb 1, 2018
@lukeed
Copy link
Owner

lukeed commented Feb 1, 2018

Thank you @bl4ckm0r3~! 🎉

@bl4ckm0r3
Copy link
Contributor Author

thank you @lukeed very nice library!
ps i have slept on it...like json better :D

@lukeed
Copy link
Owner

lukeed commented Feb 1, 2018

Haha, great!!

@StephenGregory StephenGregory mentioned this pull request Feb 7, 2018
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