Skip to content
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.

Sign up for GitHub

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

Jump to bottom

create vee-eight-4.9 branch #4722

Merged
merged 9 commits into from
Feb 8, 2016
Merged

create vee-eight-4.9 branch #4722

merged 9 commits into from
Feb 8, 2016

Conversation

targos
Copy link
Member

@targos targos commented Jan 16, 2016

The V8 team just cut the 4.9 branch so I started working on its integration.

That's still a work in progress but I'm creating the PR now because I have issues with a failing test and we will have to discuss about what we do with the V8 APIs that have been deprecated in 4.9 (as an example, I fixed the deprecation warning about String::NewFromOneByte here).
The branch cannot be built without manually copying the trace_event dependency (see nodejs/build#304)

cc @ofrobots @nodejs/v8

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Jan 16, 2016
@ofrobots
Copy link
Contributor

@targos Can you cherry pick e9f499e and b83a33f as the solution for nodejs/build#304?

@ofrobots
Copy link
Contributor

Oh, I see that you already added that commit to your list. You can ignore the previous message.

@ofrobots
Copy link
Contributor

@targos I updated to 4.9.385.11, and added some commits here: https://github.com/ofrobots/node/commits/vee-eight-4.9. With this the tests are now fully passing. There are still some deprecation warnings that need to be ironed out, but this is looking good to land into the vee-eight-4.9 branch.

Your commits LGTM.

@targos
Copy link
Member Author

targos commented Jan 24, 2016

@ofrobots thanks, I picked up your branch here

@targos
Copy link
Member Author

targos commented Jan 25, 2016

thanks @ofrobots. I updated the proxy tab-completion test.
Tests are passing locally. Let's try a CI: https://ci.nodejs.org/job/node-test-commit/1913/

@ChALkeR
Copy link
Member

ChALkeR commented Jan 25, 2016

#4869

@ChALkeR ChALkeR mentioned this pull request Jan 25, 2016
Closed
4 tasks
@rvagg
Copy link
Member

rvagg commented Jan 26, 2016

/cc @nodejs/addon-api, also see #4869, I haven't looked at this but it sounds like there's enough movement that it impacts NAN. Node.js v6 will be out with at least V8 v4.9, hopefully V8 v5.0.

@ofrobots
Copy link
Contributor

Part of the problem here has been that we are only now getting to integrating V8 4.9 with Node.js. V8 4.9 is not stable yet, so if there is a deal breaker deprecation, let's provide the feedback to the V8 team and see what can be one.

@targos
Copy link
Member Author

targos commented Jan 26, 2016

I pushed two new V8 deprecation-related commits.

Perhaps we could go with a review of the PR as it is now, merge it into vee-eight-4.9 and continue the integration from there ?

@ofrobots
Copy link
Contributor

@targos Your commits LGTM. Someone else will have to review mine.

@targos
Copy link
Member Author

targos commented Jan 29, 2016

ping @nodejs/v8 for additional review

@ofrobots
Copy link
Contributor

ofrobots commented Feb 1, 2016

@targos perhaps you can review my commits.

@bnoordhuis
Copy link
Member

Style nits but otherwise LGTM.

@targos
Copy link
Member Author

targos commented Feb 6, 2016

CI: https://ci.nodejs.org/job/node-test-commit/2134/

ofrobots and others added 9 commits February 8, 2016 12:46
@ofrobots @targos
deps: upgrade to V8 4.9.385.18
fdadc02 
Pick up the current branch head for V8 4.9
v8/v8@1ecba0f

PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@ofrobots @targos
deps: edit v8 gitignore to allow trace_event copy
c0e11d8 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@ofrobots @targos
deps: add V8 DEP trace_event
4b65870 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos
test: fix tests after V8 upgrade
c51d488 
- An error message changed for undefined references
- `let` is now allowed in sloppy mode
- ES2015 proxies are shipped and the `Proxy` global is now a function

PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
@ofrobots @targos
test: fix proxy tab-completion test
9796b0c 
Proxies support is now complete in V8. The tests needed slight modification to
match the spec implementation.

PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@ofrobots @targos
lib: wrap tick_processor scripts in IIFE
4c1433d 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos
src: fix String::NewFromOneByte deprecation warnings
01de50d 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
@targos
src: fix TryCatch deprecation warnings
4a341b0 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
@targos
src: fix NewInstance deprecation warnings
e4ad3ab 
PR-URL: nodejs#4722
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
@addaleax addaleax mentioned this pull request Apr 28, 2016
Closed
demurgos added a commit to demurgos/browser-compat-data that referenced this pull request Apr 6, 2018
@demurgos
Update nodejs compat for javascript.function.rest_parameters
380fd80 
This commit documents Node.js compatibility for function rest
parameters. This mainly depends on V8 but I tracked the relevant Node
versions, commits and PRs.

Flagged support (V8 4.4):

- [Node.js changelog entry for version 3.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_IOJS.md#2015-08-04-version-300-rvagg)
- [Pull Request](nodejs/node#2022)
- [Commit, with anchor to relevant line](nodejs/node@70d1f32f56#diff-b2e04de0d939630d882245c2243e7e47R200)

Stable support (V8 4.7):

- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4106)
- [Commit, with anchor to relevant line](nodejs/node@8a43a3d#diff-b2e04de0d939630d882245c2243e7e47R217)

Flag removal (V8 4.9):
- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4722)
- [Commit, with anchor to relevant line](nodejs/node@069e02a#diff-b2e04de0d939630d882245c2243e7e47L221)
demurgos added a commit to demurgos/browser-compat-data that referenced this pull request Apr 6, 2018
@demurgos
Update nodejs compat for javascript.function.rest_parameters
d84b7da 
This commit documents Node.js compatibility for function rest
parameters. This mainly depends on V8 but I tracked the relevant Node
versions, commits and PRs.

Note that I used `4` for the version of the flag addition because `3`
corresponds to the io.js fork and is not recognized as a valid `nodejs`
version by the linter.

Flagged support (io.js 3.0.0, V8 4.4):

- [Node.js changelog entry for version 3.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_IOJS.md#2015-08-04-version-300-rvagg)
- [Pull Request](nodejs/node#2022)
- [Commit, with anchor to relevant line](nodejs/node@70d1f32f56#diff-b2e04de0d939630d882245c2243e7e47R200)

Stable support (Node.js 6.0.0, V8 4.7):

- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4106)
- [Commit, with anchor to relevant line](nodejs/node@8a43a3d#diff-b2e04de0d939630d882245c2243e7e47R217)

Flag removal (Node.js 6.0.0, V8 4.9):
- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4722)
- [Commit, with anchor to relevant line](nodejs/node@069e02a#diff-b2e04de0d939630d882245c2243e7e47L221)
demurgos added a commit to demurgos/browser-compat-data that referenced this pull request Apr 6, 2018
@demurgos
Update nodejs compat for javascript.function.rest_parameters
3346b89 
Flagged support:

- [Node.js changelog entry for version 3.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_IOJS.md#2015-08-04-version-300-rvagg)
- [Pull Request](nodejs/node#2022)
- [Commit, with anchor to relevant line](nodejs/node@70d1f32f56#diff-b2e04de0d939630d882245c2243e7e47R200)

Stable support:

- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4106)
- [Commit, with anchor to relevant line](nodejs/node@8a43a3d#diff-b2e04de0d939630d882245c2243e7e47R217)

Flag removal:
- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4722)
- [Commit, with anchor to relevant line](nodejs/node@069e02a#diff-b2e04de0d939630d882245c2243e7e47L221)
demurgos added a commit to demurgos/browser-compat-data that referenced this pull request Apr 6, 2018
@demurgos
Update nodejs compat for javascript.function.rest_parameters
5cca4a2 
This commit documents Node.js compatibility for function rest
parameters. This mainly depends on V8 but I tracked the relevant Node
versions, commits and PRs.

Flagged support (V8 4.4):

- [Node.js changelog entry for version 3.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_IOJS.md#2015-08-04-version-300-rvagg)
- [Pull Request](nodejs/node#2022)
- [Commit, with anchor to relevant line](nodejs/node@70d1f32f56#diff-b2e04de0d939630d882245c2243e7e47R200)

Stable support (V8 4.7):

- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4106)
- [Commit, with anchor to relevant line](nodejs/node@8a43a3d#diff-b2e04de0d939630d882245c2243e7e47R217)

Flag removal (V8 4.9):
- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4722)
- [Commit, with anchor to relevant line](nodejs/node@069e02a#diff-b2e04de0d939630d882245c2243e7e47L221)
@demurgos demurgos mentioned this pull request Apr 6, 2018
Merged
Elchi3 pushed a commit to mdn/browser-compat-data that referenced this pull request Apr 11, 2018
@demurgos @Elchi3
Update nodejs compat for javascript.function.rest_parameters (#1744)
8292290 
* Update nodejs compat for javascript.function.rest_parameters

This commit documents Node.js compatibility for function rest
parameters. This mainly depends on V8 but I tracked the relevant Node
versions, commits and PRs.

Note that I used `4` for the version of the flag addition because `3`
corresponds to the io.js fork and is not recognized as a valid `nodejs`
version by the linter.

Flagged support (io.js 3.0.0, V8 4.4):

- [Node.js changelog entry for version 3.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_IOJS.md#2015-08-04-version-300-rvagg)
- [Pull Request](nodejs/node#2022)
- [Commit, with anchor to relevant line](nodejs/node@70d1f32f56#diff-b2e04de0d939630d882245c2243e7e47R200)

Stable support (Node.js 6.0.0, V8 4.7):

- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4106)
- [Commit, with anchor to relevant line](nodejs/node@8a43a3d#diff-b2e04de0d939630d882245c2243e7e47R217)

Flag removal (Node.js 6.0.0, V8 4.9):
- [Node.js changelog entry for version 6.0.0](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V6.md#2016-04-26-version-600-current-jasnell)
- [Pull Request](nodejs/node#4722)
- [Commit, with anchor to relevant line](nodejs/node@069e02a#diff-b2e04de0d939630d882245c2243e7e47L221)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@targos @ofrobots @ChALkeR @rvagg @bnoordhuis