-
Notifications
You must be signed in to change notification settings - Fork 6.2k
chore: move to patches #4997
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
chore: move to patches #4997
Conversation
0c272b0 to
44284b9
Compare
9bf1bfd to
d6a4f33
Compare
This is not a straight conversion from our existing fork modifications, I streamlined a bunch of things, removed some things that become unnecessary in 1.64, and moved what I could out of the patches.
44d94e0 to
a35591a
Compare
Codecov Report
@@ Coverage Diff @@ ## main #4997 +/- ## ========================================== - Coverage 71.58% 71.30% -0.29% ========================================== Files 29 30 +1 Lines 1675 1683 +8 Branches 373 373 ========================================== + Hits 1199 1200 +1 - Misses 405 413 +8 + Partials 71 70 -1
Continue to review full report at Codecov.
|
c939a7f to
45dd14b
Compare
a681fc1 to
5ba8dde
Compare
I think I got everything. Let me know if any patches appear to be missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think overall, this looks awesome! Most of my comments are non-blocking, but a few things we need to change for this to be ready.
I'm also going to test this locally and follow the docs to make sure it sounds good
Thanks for taking this on! 🙌🏼
d6231d7 to
6daae5c
Compare
Re: failing CI checks
- [non-blocking] Docs preview: @BrunoQuaresma any ideas why this is failing on forks? Might have to re-introduce skip
- Code scanning - not something new with this code so I will dimiss
- codecov - 0.29% decrease. I don't want to block this so @code-asher what if you follow-up with some new tests in a separate PR? (just to cover difference)
Testing
Tested locally in dev mode and works very smoothly!
Tested a build on macOS and that worked as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!!!
This will be easier to maintain than to have it as a patch.
Using a flag means we will not need to patch it out. I think this is new from 1.64?
This way we do not have to patch it.
Instead of the root one. This contains fewer dependencies.
This way we will not have to patch Code to make this work and I think it makes sense to let Code handle the request. If we do want to handle errors we can do it cleanly by patching their error handler to throw instead.
This way we will not have to patch it.
- Switch submodule to track upstream
- Add quilt to the process
- Add patches
The node-* ignore was ignoring one of the diffs so I removed it. This
was added when we were curling Node as node-v{version}-darwin-x64 for
the macOS build but this no longer happens (we use the Node action to
install a specific version now so we just use the system-wide Node).
Previously it was only ignoring linux-x64.
Patching is required first because we remove the .yarnrc which affects the yarn install and `patch` should be `push`.
6daae5c to
16834c1
Compare
* Move integration types into code-server
This will be easier to maintain than to have it as a patch.
* Disable connection token
Using a flag means we will not need to patch it out. I think this is
new from 1.64?
* Add product.json to build process
This way we do not have to patch it.
* Ship with remote agent package.json
Instead of the root one. This contains fewer dependencies.
* Let Code handle errors
This way we will not have to patch Code to make this work and I think it
makes sense to let Code handle the request.
If we do want to handle errors we can do it cleanly by patching their
error handler to throw instead.
* Move manifest override into code-server
This way we will not have to patch it.
* Move to patches
- Switch submodule to track upstream
- Add quilt to the process
- Add patches
The node-* ignore was ignoring one of the diffs so I removed it. This
was added when we were curling Node as node-v{version}-darwin-x64 for
the macOS build but this no longer happens (we use the Node action to
install a specific version now so we just use the system-wide Node).
* Use pre-packaged Code
Uh oh!
There was an error while loading. Please reload this page.
Still a bit to go but opening the PR now for hype.
Closes #3462
Closes #4687
Fixes #4826 (I changed the update check code a bit so this is now fixed)
Fixes #4994