merge queue: checking #1837 on main (f72f557) - #1846
Closed
mergify[bot] wants to merge 2 commits into
Closed
mergify[bot] wants to merge 2 commits into
mergify[bot] wants to merge 2 commits into
Conversation
`Theme::detect()` read `NO_COLOR`, `FORCE_COLOR` and `CLICOLOR_FORCE`
on every call, and it is called from production paths in
`mergify-freeze`, `mergify-queue`, `mergify-events` and
`mergify-stack`. So any test in any of those crates read the process
environment, on whatever thread it happened to run on, just by
rendering a themed line. That is the read half of the data race this
stack is about: the tests in `mergify-ci`, `mergify-core`,
`mergify-cli` and `mergify-stack` mutate the process environment
while these read it.
Those three reads could never change the answer inside a test binary
anyway. `resolve_enabled` returns `false` for a `None` choice
whatever they say, and only `mergify-cli`'s entry point ever records
a choice, which the doc comment on `set_color_choice` already said. A
test harness read three environment variables to compute a value it
had already decided.
`mergify-tui` now takes the resolved choice and never reads the
environment. `--color always|never` still wins, `NO_COLOR` still
beats `FORCE_COLOR` / `CLICOLOR_FORCE`, and `auto` with neither still
defers to the TTY. The precedence moves to `resolve_color_choice` in
the binary, next to the clap enum it maps from, which is also where
the workspace's environment funnel lives. `mergify-tui` cannot use
that funnel: it is `mergify-core`, and this crate deliberately stays
dependency-light.
Two behaviour fixes come with it, both in the same three variables:
Exported-but-empty no longer counts as set. A workflow writing
`NO_COLOR: ${{ inputs.no_color }}` with no input exports the empty
string, and every `mergify` in that job lost color with no way back
but `--color always`. <https://no-color.org> says "present and not an
empty string"; so does every other variable this CLI reads.
The log stream obeys the same decision. `init_tracing` was choosing
ANSI from `stderr().is_terminal()` alone, so `NO_COLOR=1` and
`--color never` gave plain stdout and a `-vv` stream still full of
escape sequences, which is the one output people redirect to a file.
The remaining visible difference is *when* the variables are read:
once at startup instead of at every `Theme::detect()`. Nothing
mutates the environment mid-run, so the answer is the same either
way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Change-Id: Ib099b7044861960ca2c9e85f443072914b333c20
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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#1837 is queued for merge on branch main (f72f557).
This pull request has been created by Mergify to check the mergeability of #1837.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:github-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]Enforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gateRequired conditions to stay in the queue:
base=maingithub-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]label!=manual mergeEnforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gate