-
Notifications
You must be signed in to change notification settings - Fork 164
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 #759 Drop support for Node 12 #763
Fix #759 Drop support for Node 12 #763
Conversation
Thanks for the PR. You may want to update https://github.com/GoogleChromeLabs/bubblewrap/blob/main/packages/cli/src/lib/Cli.ts#L39-L42 too. |
@andreban We are increasing the minimum supported Node.js version only because of the Should we update Should we add Should we update https://github.com/GoogleChromeLabs/bubblewrap/blob/main/packages/cli/src/lib/Cli.ts#L39-L42? |
By dropping support to Node 12 we'll stop testing the code against it and ensuring using APIs that work on Node 12. Right now, the whole application will still be compatible but, over time, it's likely that it will stop working on Node 12. I'd rather clarify this to users in documentation / code now, rather than surprise people when it breaks. |
We'd probably want to update the CI too: https://github.com/GoogleChromeLabs/bubblewrap/blob/main/.github/workflows/nodejs.yml#L12 |
@andreban BTW, what about the |
@andreban And unfortunately you have not answered the question, should we add If so, should we also create an |
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.
LGTM, Thank you!
No description provided.