Skip to content

Conversation

@godrei
Copy link
Contributor

@godrei godrei commented Jan 30, 2023

This PR stops rsync printing directly to the standard output.

This is needed because the Bitrise CLI uses CopyFile and CopyDir functions for activating local Steps.
When rsync fails, it directly prints an error to the stdout, which messes up the CLI's structured logging.

@godrei godrei marked this pull request as ready for review January 30, 2023 13:26
Copy link
Contributor

@ofalvai ofalvai left a comment

Choose a reason for hiding this comment

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

Looks good, but why didn't you modify the original RunCommand() method instead? That would solve all other possible logging issues, no?

@godrei
Copy link
Contributor Author

godrei commented Jan 30, 2023

@ofalvai this is the V1 command package, still in use by tools and probably by steps too and I wanted to reduce the blast radius.

@godrei godrei requested a review from vshah23 January 30, 2023 16:52
@godrei godrei merged commit b60cb41 into v1 Jan 31, 2023
@godrei godrei deleted the CI-773-rsync-output branch January 31, 2023 06:55
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.

4 participants