chore(git-node): avoid dealing with patch files for landing

[addaleax]

Working with patch files is, on the one hand, unclean because it requires manual parsing to figure out e.g. the commit message subjects, and on the other hand, poses actual problems when dealing with binary files.

Clean the code up by fetching the original commits themselves and applying those commits directly.

This also resolves some TODO comments, e.g. we can now check whether the commits that are to be applied match the ones reported through the GitHub API.

---

I’m not sure what kind of testing this patch needs? I’ve tried it out manually on one PR so far, but there doesn’t appear to be anything automated in place here.

[codecov[bot]]

Codecov Report

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

@@           Coverage Diff           @@
##           master     #486   +/-   ##
=======================================
  Coverage   83.13%   83.13%           
=======================================
  Files          34       34           
  Lines        1696     1696           
=======================================
  Hits         1410     1410           
  Misses        286      286           

---

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 3c3712a...947ec98. Read the comment docs.

[mmarchini]

Thank you! I always struggled to understand why we used patches instead of fetching the PR commits.

I’m not sure what kind of testing this patch needs? I’ve tried it out manually on one PR so far, but there doesn’t appear to be anything automated in place here.

I think most of our landing process is untested and doesn't have the testing infrastructure it needs in place, so relying on manual tests is the way to go for now.

[mmarchini]

This will conflict with #473

[addaleax]

This will conflict with #473

Since I’ll not be involved in Node.js as much in the future, just consider this something that bugged me for quite a while and that I wanted to provide a fix for, but that I’ll probably not be involved in merging long-term. If you want to make changes, feel free to also just push to this branch :)

[lundibundi]

Also, I'm fine with landing this first or helping merge if my PR lands first since my PR is much simpler anyway and I always find git-am very confusing when there are conflicts (like what do I change to fix them? :smile: ).

[mmarchini]

Let's land the autorebase first as it should allow the commit-queue to land some PRs it can't today. I'm happy to rebase this and fix the conflicts after we land the autorebase.

[mmarchini]

@addaleax I rebased and fixed the conflicts locally, let me know if I can force-push.

[addaleax]

@mmarchini You can generally do whatever you think is best on any of my branches :)

[mmarchini]

Force pushed to fix conflicts :)

[addaleax]

This can be merged, right? :)

[mmarchini]

Yeah, I just wanted to make a release first (but I don't have npm publish rights, so I'm waiting for someone else to do it).

[mmarchini]

Will try to rebase and land this weekend :)

[addaleax]

Resolved another (trivial) conflict :)