Latest version of code (New package)

[Sukhmai]

Hi all, with permission from the owner of the repository, I have published a npm package with the latest version of the code. Currently the poker-evaluator package is behind the GitHub repository. I was also permitted to add a license. If anyone has suggestions for features, or wants to make a pull request, I'm more than happy to work on updating the repository.

The new npm package: https://www.npmjs.com/package/poker-evaluatehand The new repository: https://github.com/Sukhmai/poker-evaluatehand

Hopefully this helps everyone.

[rorymcgit]

Hi Sukhmai,

This library has several npm packages now.

• Chenosaurus (original lib) https://www.npmjs.com/package/poker-evaluator (typing in https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/poker-evaluator/index.d.ts)

• Canvaskisa fork: https://www.npmjs.com/package/latest-poker-evaluator (I can't see any difference between this and Chenosaurus')

• My (rorymcgit) fork + rewrite in Typescript: https://www.npmjs.com/package/poker-evaluator-ts (I split out some of the constants so I could use their types in some frontend code that consumed them. I also added tests and updated some es5->es6 code)

If your desire is to add a licence to it I would suggest opening a pull request from your fork to this repo, instead of adding yet another similar package to the npm registry. This repo has the benefit of having been battle tested as well as having DefinitelyTyped types for users using Typescript to rely on.

As an aside, I would be happy to merge my changes into this repo and remove my poker-evaluator-ts package too.

[Sukhmai]

Hi rory, I emailed the owner of the repository, and I don't think he wants to be the maintainer of this code anymore. I'd be more than happy to delete my npm package, and have your package be the defacto latest version. My only concern is that I'm not using typescript in my application. I'm assuming I can still import your package and work with it in plain js?

I'll also add a pr for a MIT license to your repository, who gave permission through email, so that people can know that they use the package freely.

[Sukhmai]

Hi rory, I've been testing your package for a bit, and you've made a few breaking changes to the code.

1. You now export a class with evalHand() attached. This probably is the better way to do things, but it means a person can't directly upgrade from this repo to yours. It's obviously just a few line changes to update to your way of doing things, but I'm hesitant to recommend something that breaks existing code.
2. You got rid of the check if the input is string or int (your force cards.toLowerCase()). My application stores cards as ints 1-52, which is viable in the original package and breaks in yours. Because of these two problems, I'm hesitant to make your package the defacto standard. Ideally whatever is the new standard doesn't break/reduce existing functionality.

[rorymcgit]

Thanks for testing it out Sukhmai!

1. Correct, the dat file is loaded on instantiation of the class. This defers from the existing API as outlined in the README Looking at it with some fresh eyes, it does not need to happen and could instead NOT defer from the existing API, so I will change this, making evalHand() a static method and load the ranks data outside of the class.

2. I did not realise usage included passing the integer values as arguments, and thought that was only for protection before using #toLowerCase. Thanks for pointing this out, should be trivial to add back in.

My suggested next steps... If @chenosaurus is unwilling to continue maintaining the npm package and you are in contact with him I suggest you ask him to hand over ownership to you (providing you want to maintain it). This would involve @chenosaurus transferring ownership on npm https://docs.npmjs.com/transferring-a-package-from-a-user-account-to-another-user-account. You would then republish to that name from your fork.

My focus will be on updating my fork to re-align with the existing the api and accept integer arguments. I'll then open a PR into your fork and if all good, deprecate the poker-evaluator-ts package.

[Sukhmai]

That sounds great! I'll try to reach out and get the ownership transferred. Thank you for putting so much effort into improving the project.

As a sidenote: you've obviously contributed significantly more to the project than me, so if you want ownership I can also transfer the package to you. If that's not something you're interested in, I'd be happy to maintain it.

[rorymcgit]

I have a PR open in DefinitelyTyped updating the types to allow number input. I also removed evalCard from the types as this is only used internally in the package as far as I know. @Sukhmai If you could review it, I'd appreciate that!

I updated my fork as follows:

• re-aligned API with original library, so you can import/require PokerEvaluator and do PokerEvaluator.evalHand(...) instead of having to instantiate the class first (all methods are now static and the classes have no state).
• accepts number arguments now. It should be noted that for 3 card hands/input number arguments are not/have never been supported in the original library. I have not yet changed the code in my fork to accept them, but instead it throws an error explaining the limitation.

FYI I have not yet published an update to my package, but will do that after some more testing.

[Sukhmai]

Hey rory I checked out your changes and they seem good. I added a PR to your repo. I also was added as a maintainer to the poker-evaluator npm package, so if all goes well we can soon update poker-evaluator with the typescript definitions.