Skip to content

fix: use git apply not system apply #432

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

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

codebytere
Copy link
Member

Closes #421.

Switches to git apply vs system apply, since the available options for system apply differ between systems.

cc @targos

@codecov
Copy link

codecov bot commented May 30, 2020

Codecov Report

Merging #432 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #432   +/-   ##
=======================================
  Coverage   77.13%   77.13%           
=======================================
  Files          21       21           
  Lines        1496     1496           
=======================================
  Hits         1154     1154           
  Misses        342      342           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75abd3a...3fb6b9b. Read the comment docs.

@codebytere codebytere force-pushed the fix-apply-on-macos branch from 87ca7c2 to bb4a1c4 Compare May 30, 2020 16:38
@targos
Copy link
Member

targos commented May 31, 2020

There is some regression. For example with this commit: v8/v8@50af617

git node v8 backport 50af61718d49c4f04d0839476c78e3b305d03de4 succeeds with the current implementation and doesn't work with this PR (it asks to resolve merge conflicts but there are no changed files).

@codebytere codebytere force-pushed the fix-apply-on-macos branch from bb4a1c4 to 96d2b66 Compare May 31, 2020 17:30
@codebytere
Copy link
Member Author

@targos should be fixed up now - working with the supplied commit.

@codebytere codebytere marked this pull request as ready for review June 1, 2020 15:24
@codebytere codebytere requested a review from targos June 1, 2020 15:24
@codebytere codebytere force-pushed the fix-apply-on-macos branch from 96d2b66 to 3fb6b9b Compare June 1, 2020 15:25
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@codebytere codebytere merged commit 65fb311 into nodejs:master Jun 2, 2020
@codebytere codebytere deleted the fix-apply-on-macos branch June 2, 2020 15:39
@targos
Copy link
Member

targos commented Jun 3, 2020

It broke one of the V8 commands:

https://ci.nodejs.org/job/node-update-v8-canary/1366/display/redirect

05:56:25] Commit V8 update [started]
[05:56:25] Commit V8 update [failed]
[05:56:25] → args.unshift is not a function
✖ args.unshift is not a function

I'll have a look later if I have time.

@codebytere
Copy link
Member Author

@targos ah dang i know what it is - i missed two files when i was grepping. Fix up in a bit.

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.

git node backport v8 does not work on macOS
2 participants