-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Improved Windows compatibility #15372
Conversation
IMO, it is fine to add this to this PR as the changes are not testable in isolation and should be just covered by regression testing in any form, but will wait for opinions of maintainers. |
I would consider @giuseppe the closest we have to an SME on compression, so I'll ask him to take a look at the xz/xzcat change; remaining changes LGTM. I think we're good to merge without tests, given everything here should already be exercised by the /approve |
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.
LGTM
@arixmkii You need to rebase and repush. |
LGTM |
Signed-off-by: Arthur Sengileyev <[email protected]>
Rebased on latest main. |
One test failure, assuming it's a flake, restarting /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arixmkii, giuseppe, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Extracting some work done during experiments implementing #13006
First it replaces hardcoded "/dev/null" with os specific variant provided by Go lang.
os.DevNull
was used in other places, so, I assume this is just code cleanup change.Next it changes use of xzcat to xz as default windows build doesn't have xzcat (in a form of application copy or symlink (not supported on that platform).
From the docs https://linux.die.net/man/1/xzcat "xzcat is equivalent to xz --decompress --stdout." So, it should have no functional changes and preserve the bahavior. Used "-d" and "-c" short keys instead of full ones as there was use of "-k" in short form already.
[NO NEW TESTS NEEDED]
Does this PR introduce a user-facing change?