Skip to content

Conversation

@jcristau
Copy link
Contributor

We were sending stdout and stderr into the same stream and then attempting to parse as if it were a list of paths. That doesn't go awesomely well when there's actual error output mixed in there.

@jcristau jcristau requested a review from a team as a code owner October 23, 2025 11:35
@jcristau
Copy link
Contributor Author

I've split out the cleanup part in its own commit with its own commit message/explanation. Thanks for the comments here.

@jcristau jcristau enabled auto-merge (rebase) October 23, 2025 13:48
@jcristau jcristau disabled auto-merge October 23, 2025 14:24
Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lint issue to fix but this lgtm otherwise

We were sending stdout and stderr into the same stream and then
attempting to parse as if it were a list of paths.  That doesn't go
awesomely well when there's actual error output mixed in there.
@jcristau jcristau force-pushed the git-clean-stderr branch 2 times, most recently from 066547d to 919681d Compare October 24, 2025 15:54
This was copied from `run_command` in
8d7d8b1, but some stuff doesn't make
sense here:
- we don't stream the output to our log so we don't need to disable
  buffering or go through a TextIOWrapper
- we don't want to pass our stdin to the command

Instead we can use subprocess.run and let it capture stdout and check
the returncode.
@jcristau jcristau enabled auto-merge (rebase) October 24, 2025 15:58
@jcristau jcristau merged commit 928330f into taskcluster:main Oct 24, 2025
17 checks passed
@jcristau jcristau deleted the git-clean-stderr branch October 24, 2025 16:07
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.

2 participants