chore: remove the colour demo workflow - #659
Closed
NickJosevski wants to merge 1 commit into
Closed
Conversation
The workflow existed to show the before and after of #506 in a real CI log during review, and to prove the reviewed FORCE_COLOR=0 and per-line colour fixes. It builds three binaries from three points in history, which stops meaning anything once this merges. The behaviour it demonstrated is covered by the unit tests in pkg/output/color_test.go, which assert the precedence table with and without a TTY and that multi-line text is coloured one line at a time. Merge this into nj/fix-506 once #622 has been reviewed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Superseded. The removal is now the last commit on #622 itself (ca8bccf), so it is a single self-contained PR rather than a stack. The demo run is still viewable if anyone wants the before and after: https://github.com/OctopusDeploy/cli/actions/runs/30981377071 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleanup for #622. Base is
nj/fix-506, notmain— merge this into #622's branch once #622 has been reviewed, then #622 is ready formain.What this removes
.github/workflows/colour-demo.yml, the demo added in e06d7fe. It builds the CLI at three points in history —main, #622 as first reviewed, and #622 with the follow-up fixes — and printsoctopus --helpunder a matrix of colour environment variables, so reviewers can see the before and after in a real CI log:It served two review conversations: the
FORCE_COLOR=0behaviour YuKitsune raised, and the discovery that GitHub Actions resets colour at every line break, which meant the logo reached CI coloured but rendered white until the per-line fix in 8c95e27.Once #622 merges, the three-way history comparison stops being meaningful and the workflow is dead weight.
Why nothing is lost
Everything it demonstrated is asserted by unit tests in
pkg/output/color_test.go:TestIsColorEnabledIsColorEnabledTestColorHelpersHonourIsColorEnabledTestMultiLineTextIsColouredPerLineTestBlankLinesAreNotColouredThe workflow only ever ran on
workflow_dispatchand on pushes tonj/fix-506, so it has never touched normal CI and removing it changes no other build.If you would rather keep it
Say so on #622 and close this instead. It would need the three-way build trimmed to just the current binary and the branch filter dropped, since neither survives the merge.
🤖 Generated with Claude Code