Skip to content

ci: divide and simplify the workflows into scripts, shared heads and smaller files #1967

Closed

Description

@JarryShaw

This is a review of .github/workflows/ at a1cd9efbc. It proposes a sequence of small PRs, not a rewrite. The reports, with file:line evidence, are unit-tests.md, other-workflows.md and cross-cutting.md in the audit set. #1958's phases come first, since they edit the same files.

Findings

  • unit-tests.yml is 2056 lines, and 43% of it is comments, mostly history narrative. Moving that narrative to workflows.rst shrinks the file more than any restructure.
  • Splitting files. required-checks cannot needs: a job in another file, so separate top-level workflows would mean editing the ruleset. The workable split is reusable sub-workflows called from unit-tests.yml. That goes last.
  • Repeated inline shell. These blocks are better as util/ scripts than as composite actions, because composite inner steps cannot take timeout-minutes, and the apt caps double as hang detectors:
    • the changes classifier (byte-identical in two workflows);
    • the KTLO/experimental summary (5 copies);
    • the annotated install (3 full copies, 4 bare);
    • the apt cache block;
    • sample generation (3 copies);
    • create-release.yml's tag check, release upload, evidence checks and release_status (about 550 lines of shell).
  • cron-conda, cron-vendor and deploy-pages repeat the same schedule gate and verdict head, which fits one reusable workflow. The conda build leg is duplicated between create-release and cron-conda.
  • The Saturday gate runs three times, often on one commit: Pages at 02:00, Vendor and Conda at 10:00. A single weekly gate that the three listen on would save two gate runs a week.
  • Workflow pins. The workflow-pinning tests re-parse YAML on every call with the pure-Python loader. One module spends 33.6 of its 34.2 s parsing. A cached loader in a shared tests/_workflows.py would cut about 200 s of test time, and would replace about nine hand-written scanners.
  • File-size caps. Only project-tests (ulimit -f) and the experimental legs (prlimit) cap file size. test, integration, parity, engines, gate and the ordering legs have no cap.
  • Smaller items:
    • unreachable Windows conda steps;
    • CodeQL template boilerplate;
    • no top-level permissions in create-release.yml and CodeQL;
    • about 37 file.yml:N line citations in the docs that every change shifts;
    • nothing runs actionlint (1.7.12 handles the anchors; 1.7.7 does not).

Proposed order

  1. Cached workflow loader for the pins (test-only).
  2. util/ scripts: KTLO summary, changes classifier, coverage report, apt, install, samples.
  3. File-size caps on every pytest step.
  4. actionlint in Lint.
  5. Comment trim, job by job.
  6. Reusable cron head; weekly gate.
  7. Optional sub-workflow split of unit-tests.yml.

Items 2, 3 and 7 follow #1958 phase 1.

Decisions

  • A. Cite workflow steps by job and step name instead of file.yml:N.
  • B. One weekly gate instead of three Saturday gate runs (changes the design recorded in workflows.rst).
  • C. Move create-release.yml's shell to scripts, and share the conda leg with cron-conda. The release workflow is yours, so this is your call.
  • D. Let workflow pins require PyYAML, retiring the text scanners.

Security items are in a separate issue.

Activity

  1. added
    ciPull requests that change CI or workflow configuration (ci: subject prefix)
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 20, 2026
  2. JarryShaw commented on Oct 20, 2026

    @JarryShaw
    OwnerAuthor

    A. yes
    B. might still want each workflow focus on one single task tho. one big task can span multiple workflows but one workflow should only take one task.
    C. sure, reuse and modularise if good.
    D. sure.

  3. moved this to Needs decision in PyPCAPKiton Oct 20, 2026
  4. JarryShaw commented on Oct 20, 2026

    @JarryShaw
    OwnerAuthor

    Noted:

    • A: cite steps by job and step name.
    • B: one task per workflow; a large task may span workflows.
    • C: modularise create-release.yml, and reuse what fits.
    • D: workflow pins may require PyYAML.

    Under B, the weekly gate becomes its own workflow whose one task is gating. Vendor, Conda and Pages keep their own tasks, and each listens on the gate. Starting with step 1, the cached workflow loader, which doesn't touch #1958's files. The rest follows #1958 phase 1.

  5. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 20, 2026
  6. moved this from Needs decision to WIP in PyPCAPKiton Oct 20, 2026
  7. JarryShaw commented on Oct 20, 2026

    @JarryShaw
    OwnerAuthor

    Step 1 is on main as d850b8097, a test-only push. tests/_workflows.py parses each workflow text once per process, using CSafeLoader where available, and hands each caller a deep copy. The cache is keyed on the text, so a doctored copy parses afresh. The shared workflow() helper and the loaders of nine pins now use it, with no assertion changes. Measured locally, test_ktlo_legs_stay_green_1672 drops from 34.7 s to 0.7 s, and the migrated and helper-using modules from about 143 s to 73 s.

  8. JarryShaw commented on Oct 20, 2026

    @JarryShaw
    OwnerAuthor

    Blocked until #1973 merges. Steps 2–7 move unit-tests.yml shell into util scripts, and #1973 is still editing those same steps.

  9. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 20, 2026
  10. moved this from WIP to Blocked in PyPCAPKiton Oct 20, 2026
  11. 107 remaining items

  12. JarryShaw commented on Oct 23, 2026

    @JarryShaw
    OwnerAuthor

    Audit of the merged migrations against the inline rule (approved): no clear revert. Each script is duplicated, has real logic or a bug history, and none has a hand-run use. Three were borderline:

    • apt_packages.py seed: back inline. Its original shell was 2 lines (sudo cp plus an echo).
    • release_vendor_update.py: back inline, at 19 lines in a single place. That also drops the util/ checkout the job needed only for the script.
    • release_tags.py owner: kept. It shares the tag-resolve code with check, which three jobs use.

    A PR for the two reverts follows.

  13. JarryShaw commented on Oct 23, 2026

    @JarryShaw
    OwnerAuthor

    Done; closing. #2093 merged at 19:16Z (d2b9b324b), so every item of the plan has landed or been dropped by ruling.

  14. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 23, 2026
  15. added this to the PyPCAPKit v1.5 milestone on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ciPull requests that change CI or workflow configuration (ci: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions