-
Notifications
You must be signed in to change notification settings - Fork 457
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
Add ESLint and TypeScript #458
Conversation
Amazing! Will dive into this and take a look. |
f3c66ec
to
1e19d20
Compare
} | ||
|
||
if (obj instanceof Stream || readable(obj)) { | ||
//TODO: Wouldn't (obj instanceof Stream) be the only check here? Do we specifically need a Readable stream or a Stream object that's not of NodeJS Stream? |
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.
I'm not sure about this. The comment is for someone here to let me know why we do this. Or discuss later in the future if this is necessary.
Hi @leerob It's ready for review. |
@leerob I've updated the script, it should work now. |
That should work but it's failing 😓 I'm not sure why, but I'll check again. |
@leerob Updated... If it doesn't work this time, perhaps you can try clearing the cache and rerunning the build. |
It works! 🎉 |
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.
Will try to get one more review on here tomorrow, but LGTM.
Hi, @leerob just following up on this 😄 |
Planning to go ahead and merge/release #460, and then let's release this as a major version 👍 |
@leerob Do you want me to update my branch and use the reverted change, or go ahead with what I currently have with |
What do you think? I think we probably keep the breaking changes. |
I think we keep the breaking change because that way we can use it programmatically with fastify or something similar. I'll make the necessary changes and update the PR soon. |
BREAKING CHANGE: Switched to using ES2020 syntax and minimum Node engine to 14.5.0.
BREAKING CHANGE: Removes the deprecated options/arguments for starting the server. They are: - --port (-p) - --host (-h) - --unix-socket (-s)
I left it with the breaking change and updated the documentation |
Closes #457
This PR adds TypeScript and rewrites the library code using TypeScript.
Highlight: