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

Default tty to false to minimise build output #401

Closed
wants to merge 2 commits into from

Conversation

chendo
Copy link
Contributor

@chendo chendo commented Jul 4, 2023

This PR changes the default of tty to false.

Unless there's a scenario I'm not considering, it doesn't seem to make sense to have this on by default, as CI is traditionally a non-interactive environment anyway. Most command line tools change their output behaviour to be more "interactive" when a TTY is detected, but this drastically increases build output, causing build log truncation warnings.

For a single step in our pipeline, the build output size difference is staggering!

  • tty: true - 2.1MB
  • tty: false - 752KB (35% of above output, ~65% reduction)

@pzeballos
Copy link
Contributor

Hey @chendo! thanks! I believe it's a good idea, but it would require a major release which we are planning to do in the following months to make docker compose v2 the default one.
This PR will also need to update the tests 🙃 (they are currently failing)

@chendo
Copy link
Contributor Author

chendo commented Jul 7, 2023

I had a quick look at the tests but it's unclear how it works and unfortunately I won't have time to address those

@pzeballos
Copy link
Contributor

No problem! We can work on a new PR with all the tests once we are ready for the v5 of the plugin (which should be during Q4)
I'll close this one, but we'll mention you once it's ready.
Thanks again! 🙏🏻

@pzeballos pzeballos closed this Jul 7, 2023
@toote toote mentioned this pull request Jan 10, 2024
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.

None yet

2 participants