Skip to content

Stated the compiler and coverage requirements directly, and said who the pinned toolchain default serves - #718

Merged
fdesbiens merged 2 commits into
eclipse-threadx:devfrom
fdesbiens:docs/state-requirements-directly
Sep 10, 2026
Merged

Stated the compiler and coverage requirements directly, and said who the pinned toolchain default serves#718
fdesbiens merged 2 commits into
eclipse-threadx:devfrom
fdesbiens:docs/state-requirements-directly

Conversation

@fdesbiens

@fdesbiens fdesbiens commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Two comment-only changes, both about comments that told a reader something
they could not act on. No behaviour changes anywhere in this PR.

1. Requirements cited a document nobody can open

Seven comments across five files cited a maintainer-local document as the
source for two project requirements: that GCC 14 on Linux is the default
compiler, and that the coverage target is 100%. That document is not part of
this repository and is not published anywhere, so the citation gave a reader
nothing to follow -- it named a source they cannot open, in place of simply
stating the requirement.

Both requirements are real and both stay. Only the pointer goes: each comment
now states the requirement on its own terms, which is what the surrounding
prose was already doing everywhere else.

File Comment
cmake/cortex_r52.cmake the pinned reference toolchain
scripts/check_gcc.sh why the script exists
.github/workflows/gcc_check.yml why the workflow exists, and the GCC_VERSION pin
.github/workflows/r52_fvp.yml the GCC_VERSION pin
.github/workflows/regression_template.yml the coverage floor, twice

2. The pinned toolchain default did not say who it serves

cmake/cortex_r52.cmake and cmake/cortex_m52.cmake both default
ARM_TOOLCHAIN_PATH to a toolchains directory under the user's home, guarded
by an EXISTS check. Nothing said whether CI relies on that, and the natural
reading is that it does.

It does not. The three workflows that install a toolchain unpack it into the
workspace and cache it there, and r52_fvp.yml puts that directory on PATH
before configuring; scripts/check_gcc.sh passes -DARM_TOOLCHAIN_PATH at
each of its three CMake call sites. On a runner the guarded directory is
absent, the EXISTS check falls through, and the compiler comes from PATH.
The default only ever fires on a developer machine, where it is what makes a
no-flag build work.

Both comments now say that, so the default is not mistaken for a CI dependency
and not removed as dead code. cortex_r52.cmake carries the explanation and
cortex_m52.cmake refers to it, matching the cross-reference already there.

The r52 comment also claimed absolute paths mean "the build does not depend on
PATH ordering", which is only true where the pinned directory exists -- in CI
the build depends on PATH and nothing else. Qualified accordingly.

Verification

Comments only. Two paragraphs are rewrapped where the shorter text left a
ragged line.

  • scripts/check_gcc.sh --help still prints the intended block. The script
    slices its own header by line number to produce that output, so an edit
    above it is worth checking rather than assuming.
  • cmake/cortex_r52.cmake and cmake/cortex_m52.cmake both still configure.
  • The fall-through described above is real, not assumed: with HOME pointed
    at a directory holding no toolchains, cortex_r52.cmake configures against
    the arm-none-eabi-gcc found on PATH.
  • All three workflow files still parse as YAML.

…e no contributor can open

Seven comments across five files cited a maintainer-local document as the
source for two project requirements: that GCC 14 on Linux is the default
compiler, and that the coverage target is 100%. That document is not part of
this repository and is not published anywhere, so the citation gave a reader
nothing to follow -- it named a source they cannot open, in place of simply
stating the requirement.

Both requirements are real and both stay. Only the pointer goes: each comment
now states the requirement on its own terms, which is what the surrounding
prose was already doing everywhere else.

  cmake/cortex_r52.cmake                     the pinned reference toolchain
  scripts/check_gcc.sh                       why the script exists
  .github/workflows/gcc_check.yml            why the workflow exists, and the
                                             GCC_VERSION pin
  .github/workflows/r52_fvp.yml              the GCC_VERSION pin
  .github/workflows/regression_template.yml  the coverage floor, twice

Comments only; no behaviour changes. Two paragraphs are rewrapped where the
shorter text left a ragged line. Verified that scripts/check_gcc.sh still
parses and prints its help from the header range it slices, and that
cmake/cortex_r52.cmake still configures the Cortex-R52 build.

Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
…rves

Both cmake/cortex_r52.cmake and cmake/cortex_m52.cmake default
ARM_TOOLCHAIN_PATH to a toolchains directory under the user's home, guarded by
an EXISTS check. Nothing said whether CI relies on that, and the natural
reading is that it does.

It does not. The three workflows that install a toolchain unpack it into the
workspace and cache it there, and r52_fvp.yml puts that directory on PATH
before configuring; scripts/check_gcc.sh passes -DARM_TOOLCHAIN_PATH at each of
its three CMake call sites. On a runner the guarded directory is absent, the
EXISTS check falls through, and the compiler comes from PATH. The default only
ever fires on a developer machine, where it is what makes a no-flag build work.

Both comments now say that, so the default is not mistaken for a CI dependency
and not removed as dead code. cortex_r52.cmake carries the explanation and
cortex_m52.cmake refers to it, matching the cross-reference already there.

The r52 comment also claimed absolute paths mean "the build does not depend on
PATH ordering", which is only true where the pinned directory exists -- in CI
the build depends on PATH and nothing else. Qualified accordingly.

Comments only; no behaviour changes. Verified that both toolchain files still
configure, and that the fall-through is real: with HOME pointed at a directory
holding no toolchains, cortex_r52.cmake configures against the arm-none-eabi-gcc
found on PATH.

Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
@fdesbiens fdesbiens changed the title Stated the compiler and coverage requirements instead of citing a file no contributor can open Stated the compiler and coverage requirements directly, and said who the pinned toolchain default serves Sep 10, 2026
@fdesbiens
fdesbiens merged commit 39277cf into eclipse-threadx:dev Sep 10, 2026
11 checks passed
@fdesbiens
fdesbiens deleted the docs/state-requirements-directly branch September 10, 2026 11:55
fdesbiens added a commit that referenced this pull request Sep 10, 2026
…asurements (#719)

The paragraph explaining why the SMP floor sits at 99 rather than 100 ended
with a bare reference that resolves to nothing a reader of this repository can
open. It named a source outside the tree in place of one inside it.

The real source is #677: it wrote the four tests that closed 53 of the 64
lines, took the measurements the paragraph quotes -- the 180,003 windows with
zero handovers, and the three remaining lines in tx_thread_smp_utilities.c --
and raised the floor from 98 to 99. Citing it gives the next reader somewhere
to go.

This is the same correction #718 made across five other files; this line was
missed because it sits in regression_test.yml rather than the template.

Comment only; no behaviour change.

Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
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.

1 participant