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

matrix-org/node-irc dependency not tracking upstream #175

Closed
bval opened this issue Jun 17, 2018 · 9 comments
Closed

matrix-org/node-irc dependency not tracking upstream #175

bval opened this issue Jun 17, 2018 · 9 comments

Comments

@bval
Copy link
Contributor

bval commented Jun 17, 2018

In #170 / 52fcb1c the hubot-irc adapter was moved from node-irc to a fork, and pinned to a specific ref. There appear to be two problems with this:

  1. Because the dependency is pinned to a ref, the hubot-irc adapter is no longer tracking any bugfixes or updates made to that fork.
  2. The fork that has been selected is missing vital bugfixes from upstream.

An example of one such missing bugfix is martynsmith/node-irc#452. I happen to be encountering this multiple connection error right now.

Suggested fix: move back to martynsmith/node-irc.

I'm happy to send a PR to do so. Let me know if the maintainers are open to switching back.

@notpeter
Copy link
Collaborator

I'm happy to test and review a PR.

@chrisdotcode
Copy link
Collaborator

chrisdotcode commented Aug 29, 2018

Sorry for the delay.

We should most likely not switch back to martynsmith/node-irc simply because while matrix-org's version may be missing a few bug fixes, marty's version seems to have its development halted (matrix-org's version also has more features because of this).

I forgot why I exactly I pinned to a specific ref at the time, but it was probably A Bad Idea(TM). Let's definitely undo that.

I think the best course of action going forward is still to use matrix-org's version, and help port over bugs that marty's version has resolved. I'm not worried about matrix-org halting development because they use it in production to manage 10% of connections to Freenode 🙂 .

In actuality, I've recently just gotten back from a OSS development hiatus and I believe that Hubot actually has a new version - so it might make sense to re-write hubot-irc entirely (we might not actually have another option) before they deprecate APIs we're using.

Let me know your guy's thoughts.

/cc @stephenyeargin

@stephenyeargin
Copy link
Contributor

My experience has been that v3 hasn't been disruptive for other packages/adapters. The team there was mostly "decaffeinating" it (converting everything into ES6 from CoffeeScript, cleaning up other internals rather than overhauling the APIs). We're running the current version of hubot-irc and 3.1.1 without issue.

@bval
Copy link
Contributor Author

bval commented Aug 29, 2018

@chrisdotcode Sounds like despite your hiatus you're in a better position than I am to judge which of the two forks has a brighter future. I only run one of my hubot's on IRC and it's on a private server. If you think the matrix-org version is a better long-term fit, I'm all about helping to get some bugs in it fixed.

@bval
Copy link
Contributor Author

bval commented Sep 10, 2018

I have developed a potential fix in matrix-org/node-irc#34 for the primary issue that led me to open this: hubot-irc keeping open multiple connections.

@bval
Copy link
Contributor Author

bval commented Oct 9, 2018

The folks at matrix-org have merged my PR matrix-org/node-irc#34. #177 will catch us up to the latest node-irc which should contain that fix.

@bval
Copy link
Contributor Author

bval commented Oct 23, 2018

Thanks for merging #177. I believe we are still waiting on the maintainers to cut a new release to npm before we can close this one.

@bval
Copy link
Contributor Author

bval commented May 23, 2019

It has been 1.5 years since the last release of hubot-irc and 7 months since I fixed this bug. Can we either get a new release published to npm or can myself or @stephenyeargin get added as maintainers here and on npm? Those of us using this in production would love to see this bug fixed.

@nandub
Copy link
Owner

nandub commented Apr 23, 2020

Closing since #177 was merged, and a new version was released.

@nandub nandub closed this as completed Apr 23, 2020
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

5 participants