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
add socket.io v1.7.1 & update auto-update config #9791
Conversation
@@ -12,10 +12,9 @@ | |||
"npmName": "socket.io-client", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pvnr0082t
Uh...I think this is the right npmName.
How do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PeterDaveHello
Is this is the right npmName?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it? Is there a the reason for not right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
socket.io-client looks correct for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because socket.io-client in npm directing to github repo is this.
Maybe we can modify the name
and repository
of this lib to socket.io-client, I think that will be better.
Or it will confuse socket.io with socket.io-client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, socker.io it's the main repo, we host its client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, we could never change the lib name, it'll break the current users.
LGTM |
1 similar comment
LGTM |
not at all since it needs rebase ... |
44cc9dd
to
8ff8e38
Compare
@x09326 @kennynaoh I have rebased it on the latest master branch. Please help me review it again, thank you. |
Pull request for issue: #9772
@x09326 @kennynaoh Please help me review this pull request, thank you.
close #9772