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

feat: Updated typescript types for v5 release #502

Closed
a-sync opened this issue Jan 22, 2024 · 5 comments
Closed

feat: Updated typescript types for v5 release #502

a-sync opened this issue Jan 22, 2024 · 5 comments

Comments

@a-sync
Copy link
Contributor

a-sync commented Jan 22, 2024

I created a PR draft at DefinitelyTyped for the current #master branch (future v5): DefinitelyTyped/DefinitelyTyped#68279

Any suggestion/contribution is welcome, i only submitted this issue to create awareness that this exists. (I hope thats ok.)

Once v5.x.x is released on npmjs the eslint rule: "@definitelytyped/npm-naming": "off" can be removed and the PR merged if no new feature or change is added to gamedig.

@CosminPerRam
Copy link
Member

Hey, thanks bunch for this.
#496 will require some changes when merged (soon).

@a-sync
Copy link
Contributor Author

a-sync commented Jan 22, 2024

#496 will require some changes when merged (soon).

Noted.

@CosminPerRam noticed an irregular/undocumented prop: games[].options.query_port
Is that a typo in games.js by any chance? Only used at 3 games and seems like its an offset in two cases and a port number in one. 🤔

https://github.com/gamedig/node-gamedig/blob/master/lib/games.js#L1828
https://github.com/gamedig/node-gamedig/blob/master/lib/games.js#L1837
https://github.com/gamedig/node-gamedig/blob/master/lib/games.js#L2759

@CosminPerRam
Copy link
Member

It has been mentioned some time ago that query_port isn't really used (can't remember where to pin-point), unfortunately I haven't been able to look into it but I could in time for v5 (which will be 5.0.0-beta.1 btw).
Thanks for the reminder!

@podrivo
Copy link
Contributor

podrivo commented Jan 30, 2024

I'm not sure if this is fixed and can be closed, or if we should do something else here?

@a-sync
Copy link
Contributor Author

a-sync commented Jan 30, 2024

DT merged/released it so i guess so. Other changes can go to DT directly.

On a different note: for beta releases in the npmjs registry it would be better to use the beta tag, anf leave the latest tag for the stable release.

@a-sync a-sync closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants