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

stop prepend /# before id when no namespace #2509

Merged
merged 1 commit into from
Apr 10, 2016
Merged

Conversation

tw0517tw
Copy link
Contributor

@tw0517tw tw0517tw commented Apr 1, 2016

#2451 #2405 #2475 #2491

This should fix break since b73d9be and #2239

Since I'm not using namespaces, I need someone else help to check if this would break them.

@darrachequesne
Copy link
Member

Shouldn't this be nsp.name != '/' ?... ?

Else, this might actually be a good idea, since most users actually use only one namespace (the default '/').

@tw0517tw
Copy link
Contributor Author

tw0517tw commented Apr 1, 2016

Yes, it should be nsp.name !== '/' ?....
Fixed.

@tw0517tw tw0517tw changed the title stop append /# before id when no namespace stop prepend /# before id when no namespace Apr 2, 2016
@rauchg rauchg merged commit 3c7350f into socketio:master Apr 10, 2016
@nkzawa
Copy link
Contributor

nkzawa commented Apr 10, 2016

Thanks for your PR.

We are going to change socket.id on client to the same format with server, which should solve the root cause of the problem.
Let me know if you have any ideas about that.

@tw0517tw
Copy link
Contributor Author

It's good (and important?) to have same format of socket.id on both side (again). Hope to see that release soon.

@tgabi333
Copy link

As i see this change is not included in current (v1.4.8) release, can somebody help me out why?

@tw0517tw tw0517tw deleted the patch-1 branch August 29, 2016 10:38
@jdc20181
Copy link

Sometimes things cause issues- and are tested for functionality for the
best results before released (this is in general for any project) so they
might not have tested the feature yet. Or may think it will hurt something
else. Stay paitent :)

On Monday, August 29, 2016, Gábor Tóth [email protected] wrote:

As i see this change is not included in current (v1.4.8) release, can
somebody help me out why?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2509 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMphBvHp_Txg1iegcwltsn-kXxJQupu8ks5qkrF9gaJpZM4H9xse
.

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.

6 participants