-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
cat: fix #5186 by adding explicit flush. #5256
Conversation
81ec096
to
a224846
Compare
@gim913 ping ? |
a224846
to
ca688cf
Compare
@sylvestre hi, sorry, missed previous message, will try to address this this or next week, thanks 👋 |
@gim913 ping ? :) |
Hi, sorry, life happened, and then rust update forced me to fight with llvm installation 😢 P.S. want me to rebase and/or squash the branch? |
GNU testsuite comparison:
|
@sylvestre ping? ;)
|
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.
The diff looks good to me, and could be merged.
Please rebase on top of current main
; there are many CI failures, and since this branch is roundabout 150 commits behind it, I'd like to see a (more) successful run first.
690b850
to
416ecc6
Compare
Hello, are the two failures visible above something known? |
GNU testsuite comparison:
|
Yes, the Android tests currently also fail for other PRs and hence are unrelated to this PR. Some of the other failing tests, however, are related to your PR :| |
416ecc6
to
7ff138a
Compare
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, remaining CI failures are a known issue (fixed by #6762).
#5186
See linked issue for details