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

CI: Revert recent "show build commands on terminal" change #2000

Closed
wants to merge 4 commits into from

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Feb 21, 2025

GitHub Actions UI does not handle large amounts of console output with
collapsable ::group:: sections well.
For example, UI may truncate console output if a collapsable ::group::
section gets too many log lines. In some cases, GitHub does not report
truncation at all, resulting in misleading console output. In other, UI
warns: "This step has been truncated due to its large size. Download the
full logs from the menu once the workflow run has completed."

This change reverts recent commit e5a66fc.

@kinkie kinkie changed the title Undo pushing build output to github groups Revert commit e5a66fc26d Feb 21, 2025
@rousskov rousskov changed the title Revert commit e5a66fc26d CI: Revert recent "show build commands on terminal" change Feb 21, 2025
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for posting this PR!

I have pushed a commit to finish reversal of e5a66fc: When reverting a commit, we should be reverting that commit as a whole, not undoing some of that commit changes while keeping others (even if we like some of the changes in the being-reverted commit). Some of the leftovers (removed in my commit 620f4bf) were plain wrong. Some may be a good idea but should be submitted in a dedicated PR (where they may be expanded or polished as needed, in that new context).

I adjusted PR title to become more meaningful in isolation (e.g., in a list of commit titles and/or in another branch). I adjusted PR description to state the facts without an unnecessary (and incomplete) list of those who observed those facts. I also left a bit more breadcrumbs in case we decide to revisit this topic in future PRs.

If you are OK with these adjustments, please clear this PR for merging.


Time spent so far: 7 hours and 8 minutes

@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Feb 21, 2025
@kinkie
Copy link
Contributor Author

kinkie commented Feb 22, 2025

Time spent so far: 7 hours and 8 minutes

What's worse, we're back to square one, fundamental disagreement about
what to do and all

@kinkie kinkie added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Feb 22, 2025
squid-anubis pushed a commit that referenced this pull request Feb 23, 2025
GitHub Actions UI does not handle large amounts of console output with
collapsable `::group::` sections well.
For example, UI may truncate console output if a collapsable `::group::`
section gets too many log lines. In some cases, GitHub does not report
truncation at all, resulting in misleading console output. In other, UI
warns: "This step has been truncated due to its large size. Download the
full logs from the menu once the workflow run has completed."

This change reverts recent commit e5a66fc.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Feb 23, 2025
@rousskov rousskov added the S-waiting-for-QA QA team action is needed (and usually required) label Feb 24, 2025
squid-anubis pushed a commit that referenced this pull request Feb 24, 2025
GitHub Actions UI does not handle large amounts of console output with
collapsable `::group::` sections well.
For example, UI may truncate console output if a collapsable `::group::`
section gets too many log lines. In some cases, GitHub does not report
truncation at all, resulting in misleading console output. In other, UI
warns: "This step has been truncated due to its large size. Download the
full logs from the menu once the workflow run has completed."

This change reverts recent commit e5a66fc.
@squid-anubis squid-anubis added M-failed-staging-other https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Feb 24, 2025
@rousskov rousskov removed S-waiting-for-QA QA team action is needed (and usually required) M-failed-staging-other https://github.com/measurement-factory/anubis#pull-request-labels labels Feb 24, 2025
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Feb 24, 2025
@rousskov rousskov added the S-waiting-for-QA QA team action is needed (and usually required) label Feb 24, 2025
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Feb 24, 2025
@rousskov rousskov removed the S-waiting-for-QA QA team action is needed (and usually required) label Feb 24, 2025
kinkie added a commit to kinkie/squid that referenced this pull request Feb 26, 2025
…he#2000)

GitHub Actions UI does not handle large amounts of console output with
collapsable `::group::` sections well.
For example, UI may truncate console output if a collapsable `::group::`
section gets too many log lines. In some cases, GitHub does not report
truncation at all, resulting in misleading console output. In other, UI
warns: "This step has been truncated due to its large size. Download the
full logs from the menu once the workflow run has completed."

This change reverts recent commit e5a66fc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants