Reconcile prismarine-yggdrasil with node-yggdrasil?
Closed this issue · 9 comments
This module (prismarine-yggdrasil) was extracted from node-minecraft-protocol PrismarineJS/node-minecraft-protocol#117, however there is also another Yggdrasil authentication module on NPM: node-yggdrasil, developed by @zekesonxx
Maybe it'd be worth merging these two modules to reduce code duplication/effort? prismarine-yggdrasil doesn't appear to be published on NPM yet so this would be a good time to sort this out, if we want to avoid having two yggdrasils. (Although, there is no harm in having multiple alternatives.)
Looking at node-yggdrasil, it appears to cover the functionality of prismarine-yggdrasil, but is somewhat more flexible, uses request instead of superagent, and has unit tests. Hasn't been modified in 11 months though so I don't know how updated it is (or whether it needs to be).
One immediate problem I noticed with node-yggdrasil: it is currently broken on node 0.12 (npm test fails), since its request dependency is broken on 0.12 (known issue, they're working on it). prismarine-yggdrasil works with node 0.12, and presumably iojs
Aside from a lack of documentation node-yggdrasil is basically finished,
although I think it might be missing a method or two.
On Sun, Feb 22, 2015 at 4:31 PM, deathcap notifications@github.com wrote:
One immediate problem I noticed with node-yggdrasil: it is currently
broken on node 0.12 (npm test fails), since its request dependency is broken
on 0.12
request/request@5c46172
(known issue, they're working on it). prismarine-yggdrasil works with node
0.12, and presumably iojs—
Reply to this email directly or view it on GitHub
#2 (comment)
.
@zekesonxx we could help with that 😄 .
I can confirm prismarine-yggdrasil works on iojs. The broken request dependency should get fixed very soon though, so it's not really an issue. Besides, from what I gather in the issues, it's just the tests failing, so no big deal.
All in all, I'm all for using node-yggdrasil, and I agree that there's no need to create two modules for such a simple task.
This is now done. What's the plan for this repo and the corresponding npm package?
Deleting both would seem like a good idea to me, to avoid confusion.
Ah, I guess it would be better to make sure nobody use this first.
Updating the readme to reflect that it is discontinued in favor of node-yggdrasil would be the way forward. As for the NPM package, it's there to stay. It's working anyway, why bother removing it.
Hmm ok
I would agree with this. Keep it up, but make it clear that node-yggdrasil is to be used instead.
this is long done