Skip to content
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

Remove cmd_execute_and_stream_data() returned debug message. #1525

Closed
wants to merge 3 commits into from

Conversation

BillyONeal
Copy link
Member

We have seen no reports of broken subprocess output handling, and this message is very spammy. Given that we still have a few messages that direct end users at --debug that seems reason enough to remove spammy messages like this.

We have seen no reports of broken subprocess output handling, and this message is very spammy. Given that we still have a few messages that direct end users at --debug that seems reason enough to remove spammy messages like this.
@ashlhud
Copy link

ashlhud commented Nov 11, 2024

This would have been helpful to disambiguate some errors we were seeing with our asset cache tool. +1

@autoantwort
Copy link
Contributor

autoantwort commented Nov 18, 2024

I am against removing this debug statement. I find it helpful when analyzing why things don't work as expected. But I would prefer a human readable time format :D

@BillyONeal
Copy link
Member Author

I am against removing this debug statement. I find it helpful when analyzing why things don't work as expected. But I would prefer a human readable time format :D

I think any such cases should be replaced by adding EchoInDebug to the right places rather than discussing return values of ReadFile

@autoantwort
Copy link
Contributor

At least on Unix it prints the exit code of the command that failed. And EchoInDebug would be spammy I guess 😅

@BillyONeal
Copy link
Member Author

At least on Unix it prints the exit code of the command that failed.

No, it prints the errno of the read call. This 'exit code' is not the child process' exit code.

@BillyONeal
Copy link
Member Author

Wait, this is the wrong one...

@BillyONeal BillyONeal closed this Nov 19, 2024
@BillyONeal
Copy link
Member Author

This is not the spam I thought it was; perhaps I already removed it a while ago. Thanks to @autoantwort for pointing this out.

@BillyONeal BillyONeal deleted the remove-some-debug-spam branch November 19, 2024 01:28
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.

3 participants