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

Update README.md #110

Merged
merged 4 commits into from
Mar 7, 2015
Merged

Update README.md #110

merged 4 commits into from
Mar 7, 2015

Conversation

Nonemoticoner
Copy link
Contributor

I propose such solution for #70 No need to make it more complex. Let's keep it simple.

I propose such solution for ToothlessGear#70 No need to make it more complex. Let's keep it simple.
@hypesystem
Copy link
Collaborator

Great! Thanks for the PR. I have a few notes:

  • If you are only adding a single data field, the shorter syntax would be message.addData("key1", "msg1");
  • I would prefer to show off the Sender#send method, rather than Sender#sendNoRetry. All you need to do is change the method name -- it takes the same arguments.
  • I really do think the section should appear before usage. It seems the primary source for confusion is stumbling upon code, trying to run it, and seeing it not work. Let's make it easy to try to run something.

@hypesystem
Copy link
Collaborator

Uh, and one small thing: we should try and be consistent with the code style. The other examples use this for error checking/logging:

if(err) console.error(err);
else    console.log(result);

You should probably use this to be consistent :)

@hypesystem hypesystem added this to the v0.10 milestone Feb 28, 2015
Second update according to @hypesystem wishes.
@Nonemoticoner
Copy link
Contributor Author

Done, as you pleased. I'm not that convinced to be so consistent. For me, it implies too much Ctrl+C > Ctrl+V bad practice but whatever. I believe you can merge it now.

@hypesystem
Copy link
Collaborator

Great! Remember to add yourself to the contributors list in package.json and in the README - then I'll merge :)

Just a note on the consistency of code style (that you comment on): the code people first see, using the library, will probably use the style that they will themselves adopt. I think it's significant what we show new users of the library. Does it have to be this particular convention? No. But showing them that we stick to a convention fosters a better mentality (and more readable code).

Added my nick to contributors list.
Added my name.
@Nonemoticoner
Copy link
Contributor Author

Done, you can merge ;)

hypesystem added a commit that referenced this pull request Mar 7, 2015
Update README.md to make it essier to understand how to use the library.
@hypesystem hypesystem merged commit 026e04b into ToothlessGear:master Mar 7, 2015
@hypesystem
Copy link
Collaborator

Sorry for the long wait - I got hit by the flu :(

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