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 critical error in readme.md #8

Merged
merged 1 commit into from
Apr 27, 2018
Merged

Conversation

stackola
Copy link
Contributor

No description provided.

@SusuKacangSoya
Copy link

SusuKacangSoya commented Apr 27, 2018

This is a serious error that will leave a lot of users infuriated.

I vote - pull this fix as soon as possible!

@KevinTyrrell
Copy link

Please fix this issue. Master branch is unusable in its current state.

@tahercool1
Copy link

Pls fix Thx

@VictorioBerra
Copy link

Please add unit tests for this fix.

@jaesung2061
Copy link

Since the author is gonna be confused as hell I thought I'd add the link where all the reactions are coming from.

https://www.reddit.com/r/ProgrammerHumor/comments/8f6cnb/made_my_first_big_contribution_to_open_source/

@lilypit
Copy link

lilypit commented Apr 27, 2018

Thank god someone found this failure - how could QA have missed it?

@Fallenstedt
Copy link

Repo is unusable. Please accept this MR

@ryaan-anthony
Copy link

+1

@ayudh37
Copy link

ayudh37 commented Apr 27, 2018

Google wants to know your location

@ScottBabbs
Copy link

Please merge ASAP.

@drselump14
Copy link

+1

@danielriley06
Copy link

We’ve been using this in production for months and our legal team just brought this to my (the very, very important CTO) attention. Please merge this PR or I’ll be forced to report you to the GitHub grammar police.

@TheFunkyMonk
Copy link

LGTM

@rvirji-va
Copy link

:shipit:

@jackphilippi
Copy link

+1

@gsarpy
Copy link

gsarpy commented Apr 27, 2018

Hey guys, I built a JavaScript library as a solution to this bug. It's called Your.js

@ArthurGarnier
Copy link

This bug avoid to read the file completely, merge ASAP!!

@tobiastornros
Copy link

Which font is that?

@bencoveney
Copy link

Also might want to consider:
Theres => There's
Rhobust => Robust

@Russell-IO
Copy link

Critical issue

@@ -93,7 +93,7 @@ Other popular BTC average modules require incredible amount of bandwidth at regu

This module **does not include exchange API's that do not have web sockets**. Why? Theres a billion other modules that do something similar, and perhaps this is a wake up call to any exchange that does not offer rhobust websocket support.

Choose a reason for hiding this comment

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

While we're at it, can we change "API's" to "APIs"? Thanks.

Choose a reason for hiding this comment

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

Also: rhobust -> robust

@@ -93,7 +93,7 @@ Other popular BTC average modules require incredible amount of bandwidth at regu

This module **does not include exchange API's that do not have web sockets**. Why? Theres a billion other modules that do something similar, and perhaps this is a wake up call to any exchange that does not offer rhobust websocket support.

Many developers would rather not be bothered reading through additional API documentation which, in many cases, is incomplete, hard to follow and usually lacking node.js examples. I did it for you! In node. Your welcome.
Many developers would rather not be bothered reading through additional API documentation which, in many cases, is incomplete, hard to follow and usually lacking node.js examples. I did it for you! In node. You're welcome.

### Why Should I use websockets?

Choose a reason for hiding this comment

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

Weird capitalization

@Seluj78
Copy link

Seluj78 commented Apr 27, 2018

LGTM 👍

@jaimehrubiks
Copy link

LGTM. Some automatic tests should be made before making to the master branch though.

@woolleyalan
Copy link

Is there a workaround for this for the time-being ?

Copy link
Owner

@redcap3000 redcap3000 left a comment

Choose a reason for hiding this comment

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

While this is no excuse; in my defense I was using a very terrible keyboard; it has been very recently laid to rest. I appreciate your contribution and encourage more! Welcome to the wonderful world of free and open source.

@redcap3000 redcap3000 merged commit dfd42c6 into redcap3000:master Apr 27, 2018
@nth-commit
Copy link

This is a showstopper for my organisation. My boss is really riding me hard on this one, I assured him that crypto-socket was a stable library.

@redcap3000
Copy link
Owner

In all seriousness I was actually fired for critical errors such as these long long ago while working in the (Silicon) Valley; but that was not a programming job. This is merely a hobby for me, I appreciate the attention to detail, language is something I struggle with daily. I have since banished myself from civilization as I am essentially unemployable in every industry.

@iamvinny
Copy link

It was totally unusable before, thank god it got fixed, good job @stackola.

@CAYdenberg
Copy link

No one's said it yet ...

298ay3

@redcap3000
Copy link
Owner

Attention grammar police; there is a new PR that desperately needs your attention, I feel if we all work together we can make these new changes a reality. #9 ; I am very suspicious of the changes and require your assistance to validate them. I am somewhat opposed to the use of the oxford comma in technical documentation but I am open to the discussion so long as there are sufficient reviewers. While you're here please star the repo; perhaps it may reach trending status so that we may garner additional attention for this fastidious work.

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