Skip to content

test(corekit): drop the dead conventions.rst candidate and the stale #912 tense in _sentinel_section #920

Closed

Description

@JarryShaw

_sentinel_section() in tests/corekit/test_sentinel_exports_unit.py still carries two leftovers from #912's move of conventions.rst, both surfaced by the cross-review of #919.

1. A dead first candidate. Lines 135-136 on 3766c3c09:

for candidate in ('docs/source/conventions.rst',
                  'docs/source/contributing/conventions.rst'):

docs/source/conventions.rst has not existed since #912 landed as 9806f16aa, so the first iteration's path.is_file() is always false. The loop still works — it falls through to the real path — but it is dead code that reads as though both locations were live.

2. A stale tense in the docstring. Lines 129-131 describe the move as in progress:

the move to :file:docs/source/contributing/conventions.rst that GitHub pull request #912 is making

#912 has merged. #919 corrected the :file: path on that same docstring's first line but left the tense and the dead branch beneath it.

Neither is a behaviour defect: the helper resolves the right file today, and the else: raise AssertionError fallback is correct. It is hygiene, and the reason to fix it is that a reader cannot tell the dead branch from a deliberate compatibility shim.

Blocked on #911. That issue's work has this exact file dirty in flight, editing the module docstring and the SENTINELS table. A second edit to it now would either conflict or silently revert. Unblocks when #911 merges. #919 is also still open against this file, so the clean start point is after both.

Activity

  1. added
    testPull requests that add or correct tests (test: subject prefix)
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 29, 2026
  2. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked — #911 merged as c7342cac1. The file contention is gone: tests/corekit/test_sentinel_exports_unit.py is no longer held by an open PR.

    Both leftovers this issue describes survived the merge unchanged — the dead first candidate 'docs/source/conventions.rst' in _sentinel_section()`'s fallback tuple, and the docstring still describing #912's move as in progress. #922 deliberately left the tuple alone on my instruction, and flagged it rather than editing it.

    Removing blocked.

  3. removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 29, 2026
  4. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Fixed and pushed directly to main as 5e0ec3889 — test-only, which is authorised here, so it did not need a PR.

    Both leftovers gone:

    Comment-only and docstring-only; no assertion changed. Verified: tests/corekit/test_sentinel_exports_unit.py 18 tests OK, tests/project/test_isort_clean.py OK, and grep finds zero occurrences of the pre-move path anywhere under pcapkit/, tests/ or docs/ — so this closes the last of #912's trailing references rather than just the two named here.

    Closing.

  5. added this to the 1.5 milestone on Oct 6, 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

    testPull requests that add or correct tests (test: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions