remove support for node before v6 #1519
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awesome! I'll have a close look at little later just in case you missed
something.
…On Jan 24, 2017 22:45, "Brendan Kenny" ***@***.***> wrote:
fixes #449 <#449>
just basic removal of version checks and logic specifically for getting
--harmony piped into the CI system. New language features (mostly default
parameters, rest args, and [].includes()) can come later.
@XhmikosR <https://github.com/XhmikosR> if I'm missing anything :)
------------------------------
You can view, comment on, or merge this pull request online at:
#1519
Commit Summary
- remove support for node before v6
File Changes
- *M* .travis.yml
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-0>
(5)
- *M* lighthouse-cli/bin.ts
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-1>
(6)
- *M* lighthouse-cli/test/cli/index-test.js
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-2>
(13)
- *M* lighthouse-cli/test/smokehouse/smokehouse.js
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-3>
(6)
- *D* lighthouse-core/.npmrc
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-4>
(1)
- *M* lighthouse-core/index.js
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-5>
(7)
- *D* lighthouse-core/lib/environment.js
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-6>
(39)
- *M* lighthouse-core/scripts/run-mocha.sh
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-7>
(2)
- *M* lighthouse-extension/package.json
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-8>
(4)
- *M* lighthouse-viewer/package.json
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-9>
(2)
- *M* package.json
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-10>
(6)
- *M* readme.md
<https://github.com/GoogleChrome/lighthouse/pull/1519/files#diff-11>
(2)
Patch Links:
- https://github.com/GoogleChrome/lighthouse/pull/1519.patch
- https://github.com/GoogleChrome/lighthouse/pull/1519.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1519>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVVtQr9GjV1ispDAFcMK-dD7iPObXKDks5rVmLbgaJpZM4Lsw2v>
.
|
ebidel
reviewed
Jan 24, 2017
| @@ -2,11 +2,11 @@ | |||
| "name": "lighthouse-extension", | |||
| "private": true, | |||
| "engines": { | |||
| "node": ">=0.8.0" | |||
| "node": ">=6" | |||
There was a problem hiding this comment.
should this match travis's 6.9.1? or vice versa?
There was a problem hiding this comment.
yeah, I don't know how much we care about the precision on this. We're going to only be testing 6.9.1, so it makes sense that we shouldn't say we work in case important bugs were fixed before that...but on the other hand we're not testing with the latest v6, so it's also possible that some bug was fixed after 6.9.1 that materially affects execution...
ebidel
reviewed
Jan 24, 2017
| @@ -2,7 +2,7 @@ | |||
| "name": "lighthouse-viewer", | |||
| "private": true, | |||
| "engines": { | |||
| "node": ">=5" | |||
| "node": ">=6" | |||
ebidel
reviewed
Jan 24, 2017
| @@ -8,7 +8,7 @@ | |||
| "chrome-debug": "./lighthouse-cli/manual-chrome-launcher.js" | |||
| }, | |||
| "engines": { | |||
| "node": ">=5" | |||
| "node": ">=6" | |||
|
But do you need 6.9.1 specifically? If not just setting this to 6 will
prevent any confusion since CI will always have the latest 6 version anyway.
…On Jan 24, 2017 23:50, "Brendan Kenny" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lighthouse-extension/package.json
<#1519>:
> @@ -2,11 +2,11 @@
"name": "lighthouse-extension",
"private": true,
"engines": {
- "node": ">=0.8.0"
+ "node": ">=6"
yeah, I don't know how much we care about the precision on this. We're
going to only be testing 6.9.1, so it makes sense that we shouldn't say we
work in case important bugs were fixed before that...but on the other hand
we're not testing with the latest v6, so it's also possible that some bug
was fixed *after* 6.9.1 that materially affects execution...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1519>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVVtRh_dXIx6OzQ1k0b8ZMPrYFEvIGwks5rVnIYgaJpZM4Lsw2v>
.
|
Similar to how we were specifically testing 4.3.2 before, a key user is stuck with 6.9.1 for at least the immediate future. It's probably best to test that exact version rather than test with latest 6 and keep an ear out for anything that might affect us (e.g. while there's been only a few changes between 6.9.1 and latest 6.9.4, there was one destructuring case that no longer throws, and it's possible we could use it in LH in the future and cause issues for anyone stuck with the slightly older version). |
XhmikosR
approved these changes
Jan 24, 2017
ebidel
approved these changes
Jan 24, 2017
|
Ready after the resolve |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #449
just basic removal of version checks and logic specifically for getting
--harmonypiped into the CI system. New language features (mostly default parameters, rest args, and[].includes()) can come later.@XhmikosR if I'm missing anything :)