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.

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

[platform] Override CreateJob instead of PostJob #145

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

backes
Copy link
Member

@backes backes commented Jul 21, 2022

PostJob will call out to CreateJob in its default implementation, so
it's sufficient to only override CreateJob.

The default implementation of CreateJob will return a nullptr though, so
we cannot use it in V8 before Node overrides it.

PostJob will call out to CreateJob in its default implementation, so
it's sufficient to only override CreateJob.

The default implementation of CreateJob will return a nullptr though, so
we cannot use it in V8 before Node overrides it.
@backes backes force-pushed the override-create-job branch from b005bfd to 4abb09c Compare July 21, 2022 13:10
@backes
Copy link
Member Author

backes commented Jul 21, 2022

@pthier @victorgomes PTAL

@pthier
Copy link

pthier commented Jul 21, 2022

Thanks Clemens!

Just for documentation: The interface was changed in https://chromium.googlesource.com/v8/v8.git/+/1e0d18dc0b50a9221fe9955a734905eb918c232c

@pthier pthier merged commit ad446be into v8:node-ci-2022-07-20 Jul 21, 2022
victorgomes pushed a commit that referenced this pull request Aug 11, 2022
PostJob will call out to CreateJob in its default implementation, so
it's sufficient to only override CreateJob.

The default implementation of CreateJob will return a nullptr though, so
we cannot use it in V8 before Node overrides it.
pthier pushed a commit that referenced this pull request Sep 22, 2022
PostJob will call out to CreateJob in its default implementation, so
it's sufficient to only override CreateJob.

The default implementation of CreateJob will return a nullptr though, so
we cannot use it in V8 before Node overrides it.
zcbenz pushed a commit to photoionization/node_ci that referenced this pull request Apr 17, 2023
To include v8/node#145

Change-Id: I979eb03661856042e5641bf9c6f43ff335d82c37
Reviewed-on: https://chromium-review.googlesource.com/c/v8/node-ci/+/3789743
Auto-Submit: Clemens Backes <[email protected]>
Reviewed-by: Patrick Thier <[email protected]>
Commit-Queue: Patrick Thier <[email protected]>
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.

2 participants