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

PING/PONG Error! #415

Closed
Bioblaze opened this issue Oct 15, 2015 · 12 comments
Closed

PING/PONG Error! #415

Bioblaze opened this issue Oct 15, 2015 · 12 comments

Comments

@Bioblaze
Copy link

https://github.com/martynsmith/node-irc/blob/master/lib/irc.js#L227-#L233

Your Ping/Pong Replies are alittle messed up. :X Thats why it keeps disconnecting randomly @.@

o.o or am i reading that piece of code wrong... :X

@ghost
Copy link

ghost commented Oct 24, 2015

PR #418 maybe helping here.

@philip-peterson
Copy link
Contributor

The code here seems to be fine to me. What it says is that if a PING is received from the server, to reply with a PONG. The calls to self.emit() are incidental and don't really have to do with how the client functions, they're just in case other code wants to hook into the ping or pong events.

@moshmage
Copy link

motion for close?

@philip-peterson
Copy link
Contributor

I concur, but is this project even being maintained anymore? @martynsmith seems to be missing in action since over 2 months ago...

@ghost
Copy link

ghost commented Oct 29, 2015

Not completely, but close to, since #399 I've jumped to re-kickstart. I know its still used by a lot of users, but I'm trying to save it for my own selfish reasoning in order to learn git/github. I don't know how to merge or rebase yet, but I'm not sure more PR's would help because nobody is checking them (i will in the future). I've talked with Jirwin once, about 2 months ago, as far as i know he possibly might pass it on to the right person.

@moshmage
Copy link

@jnull

nobody is checking them

I've been reviewing the PRs that are not from people on the IRC (because I trust'em on the irc cof) also, @jirwin should be up and about, while away, to do the Merging.

It would be nice to have all these PRs in so we could decide WHERE/HOW to refactor this baby.

@ghost
Copy link

ghost commented Oct 29, 2015

@moshmage my mistake, ya i totally agree that the PRs would be good place to be. I think looking at the old PRs and Forks might help too, i don't have a good method/tool to do that though. The project was reverted and lost changes (around January i think), so there might be large chunks of stable code floating around?

@moshmage
Copy link

I only joined up like.. 28 days ago, don't even have the collaborator badge yet, or I'd be merging PRs left and right, lol.

On account of the lost code and forks, the forks that are not asking for a PR shouldn't be merged for respect reasons, maybe their fork has something that we shouldn't even think about; If Forkers want to merge, they'll ask, believe you me :)

on the merging method/tools: You can always try using IntelliJ (or Webstorm, whatever float your boat), and then make a pull from whatever PR/branch/etc - The IDE will spawn a "merge dialog" which will as you "accept mine", "accept theirs" or "merge" - when you click "merge" it will open a window with the files side by side, all you have to do is click the "yes, I want this change to be added in" or "no, ignore this change" icons near the lines.

@ghost
Copy link

ghost commented Oct 29, 2015

I'll look at Webstorm and IntelliJ, in the mean time I'll be messing with forks to get up on my PRs.

@philip-peterson
Copy link
Contributor

Is there any interest in maybe starting a major fork? I'd be willing to help maintain it if I could get a couple dedicated co-maintainers. I love this project but unfortunately I just don't use IRC much anymore, so my ability to push new features would be limited, but I'd be more than happy to consider merging in others' pull requests.

@ghost
Copy link

ghost commented Nov 1, 2015

I agree, this is a great project, and I'd join you on that conquest, but maybe its possible to stay on this fork? (@jirwin: perhaps a new branch?). I think staying here for the traffic would be optimal if we can become contributors (I nominate @philip-peterson lol) . Also, if people did start using "our" new fork i think they would eventually failover to 'npm install irc', its such a nooby friendly & easy to remember name.

@jirwin
Copy link
Collaborator

jirwin commented Mar 24, 2016

I believe this issue is fixed by #418 and #452.

@jirwin jirwin closed this as completed Mar 24, 2016
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

No branches or pull requests

4 participants