-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix: Add node types to typings #974
Conversation
@ffflorian What problem is this Pull Request solving? |
@shadowspan |
From the comment:
Does that mean you approve? |
No it does not mean I approve, it means I have started looking into it. If you have good links for the details of what |
You don't know what I expect 🙂 I was just wondering what you meant by your comment, sorry. Take your time! Here are the docs for In short:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with the triple slash, thanks. I think it is the Right Thing to do. As a small bonus it changes the tsc
error messages when node types are not installed, and that may help people recognise the problem quicker.
In part because we are shipping definitions ourselves now, I do not wish to list new minor contributors in the "Definitions by" please. On DefinitelyTyped that had the benefit of getting email notifications about related changes but that is no longer the case. (We have git history and GitHub contributors for author credit.)
@shadowspawn I removed the changes to the contributors line. |
Thanks @ffflorian |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Merging into |
Already had two approvals. Merged into v3 and will be included in that release. Closing to make it clear that should not be merged into master. Thank you for your contributions. |
Thanks @shadowspawn @abetomo, looking forward to v3! |
Available now as a prerelease. See #1001 |
Shipped in v3: https://github.com/tj/commander.js/releases/tag/v3.0.0 |
No description provided.