Skip to content

Conversation

@gabrielschulhof
Copy link
Contributor

No description provided.

Copy link
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NickNaso NickNaso mentioned this pull request Oct 24, 2019
15 tasks
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@legendecas
Copy link
Member

Still, I'd wonder if it's good to remove the retired LTS version from our test settings. Should the support of this library keep aligned with LTS lifecycles? Since there is no harm to keep an eye on if we break things on lower NAPI_VERSION targets.

@gabrielschulhof
Copy link
Contributor Author

@legendecas maybe we should move those to accepted failures.

@gabrielschulhof
Copy link
Contributor Author

@mhdawson @NickNaso WDYT? Should we move node/6 to allow_failures instead of removing it completely?

@NickNaso
Copy link
Member

@gabrielschulhof It make sense for me because we could know if we will introduce some kind of breakage even if on the dismissed Node.js version.

@gabrielschulhof
Copy link
Contributor Author

@NickNaso OK I changed the PR accordingly. @legendecas, @mhdawson please take another look!

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mhdawson mhdawson merged commit b42e21e into nodejs:master Oct 31, 2019
@gabrielschulhof gabrielschulhof deleted the remove-node-6 branch February 1, 2021 16:39
@gabrielschulhof gabrielschulhof restored the remove-node-6 branch February 1, 2021 16:40
@gabrielschulhof gabrielschulhof deleted the remove-node-6 branch March 7, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants