Skip to content

Apply senior-engineer review fixes across packaging, CI, and docs - #12

Merged
mahynski merged 2 commits into
mainfrom
claude/code-review-XO0JW
May 3, 2026
Merged

Apply senior-engineer review fixes across packaging, CI, and docs#12
mahynski merged 2 commits into
mainfrom
claude/code-review-XO0JW

Conversation

@mahynski

@mahynski mahynski commented May 3, 2026

Copy link
Copy Markdown
Owner

Real bugs:

  • .readthedocs.yml referenced docs/requirements.txt which is never
    committed; switch RTD to install the package's new [docs] extra so
    builds work out of the box.
  • docs/conf.py inserted ../my_package on sys.path (subpackages would
    import as top-level names); use ../ so autodoc resolves my_package.*.
  • Drop hard pin pandas==2.1.4 to >=2.1.4 to unblock numpy 2.x stacks.

Packaging:

  • Move dev tooling (pre-commit, pytest, mypy, IPython, ipykernel,
    ipywidgets, sphinx) out of runtime dependencies into [test], [docs],
    [notebook], and an aggregate [dev] extra.
  • Fix Development Status classifier (5 -> 1) and add 3.10-3.13
    classifiers to match requires-python.

CI hardening:

  • python-app.yml now runs a 3.10/3.11/3.12/3.13 matrix, installs the
    [test] extra, and uploads codecov only from the 3.12 leg with
    fail_ci_if_error=false. Added concurrency, least-privilege
    permissions, and timeout-minutes.
  • pre-commit.yml and codeql.yml gain matching concurrency / timeout /
    permissions blocks.
  • .gitlab-ci.yml synced to ruff/pytest, parallel matrix, and cobertura
    coverage report.

Template hygiene:

  • README: replace blanket sed loop with a file-extension-restricted
    find, drop the now-stale pip-compile docs workflow, document the new
    extras, expand the pre-release checklist (CITATION + classifiers),
    and clarify the dev-branch caveat for CI triggers.
  • docs/conf.py: dynamic copyright year, document how to enable
    sphinxcontrib-bibtex, and correct the misleading nbsphinx_execute
    comment.
  • tests/test_example.py: add TODO marker so the no-op suite is not
    mistaken for real coverage; rename setUpClass parameter to cls.

claude added 2 commits May 3, 2026 03:19
Real bugs:
- .readthedocs.yml referenced docs/requirements.txt which is never
  committed; switch RTD to install the package's new [docs] extra so
  builds work out of the box.
- docs/conf.py inserted ../my_package on sys.path (subpackages would
  import as top-level names); use ../ so autodoc resolves my_package.*.
- Drop hard pin pandas==2.1.4 to >=2.1.4 to unblock numpy 2.x stacks.

Packaging:
- Move dev tooling (pre-commit, pytest, mypy, IPython, ipykernel,
  ipywidgets, sphinx) out of runtime dependencies into [test], [docs],
  [notebook], and an aggregate [dev] extra.
- Fix Development Status classifier (5 -> 1) and add 3.10-3.13
  classifiers to match requires-python.

CI hardening:
- python-app.yml now runs a 3.10/3.11/3.12/3.13 matrix, installs the
  [test] extra, and uploads codecov only from the 3.12 leg with
  fail_ci_if_error=false. Added concurrency, least-privilege
  permissions, and timeout-minutes.
- pre-commit.yml and codeql.yml gain matching concurrency / timeout /
  permissions blocks.
- .gitlab-ci.yml synced to ruff/pytest, parallel matrix, and cobertura
  coverage report.

Template hygiene:
- README: replace blanket sed loop with a file-extension-restricted
  find, drop the now-stale pip-compile docs workflow, document the new
  extras, expand the pre-release checklist (CITATION + classifiers),
  and clarify the dev-branch caveat for CI triggers.
- docs/conf.py: dynamic copyright year, document how to enable
  sphinxcontrib-bibtex, and correct the misleading nbsphinx_execute
  comment.
- tests/test_example.py: add TODO marker so the no-op suite is not
  mistaken for real coverage; rename setUpClass parameter to cls.
Real bugs:
- docs/index.rst references a `modules` toctree but RTD only ran
  sphinx-build, never sphinx-apidoc, so the hosted API reference was
  empty (only suppressed by fail_on_warning: false). Add a pre_build
  job to .readthedocs.yml that runs sphinx-apidoc, and gitignore the
  generated docs/modules.rst / docs/my_package*.rst.
- docs/Makefile.txt was unreachable by `make` (wrong filename); rename
  to docs/Makefile and rewrite docs/make_docs.sh to call sphinx-build
  directly with `set -euo pipefail`, dropping the redundant second
  `make html`.

Supply-chain hardening:
- Pin every third-party GitHub Action to a 40-char commit SHA with a
  trailing version comment (actions/checkout v4.3.1, setup-python
  v5.6.0, codecov-action v5.5.4, codeql-action v3.35.3, pre-commit
  action v3.0.1). Bump codecov-action v4 -> v5 along the way.
- Group Dependabot updates so SHA bumps land as a single PR per
  ecosystem instead of one per action.

Tooling config:
- Add [tool.ruff] / [tool.ruff.lint] (target-version = py310,
  line-length = 100, E/W/F/I/B/UP) so config is in pyproject.toml
  rather than only in pre-commit.
- Add [tool.coverage.run] source = ["my_package"] and branch = true
  so coverage reflects the package, not whatever happens to import
  during tests (tests/ inflated the previous numbers to 100%).
- Bump ruff pre-commit hook v0.9.9 -> v0.15.12 and pin
  default_language_version to python3.12.

Template polish:
- Remove tests/__init__.py: pytest doesn't need it, and it makes the
  tests/ tree behave inconsistently across import modes.
- Comment out the {github_id} Zenodo badge in README.md and
  docs/index.rst so the rendered docs don't ship a broken image.
- CITATION.cff: drop placeholder doi / date-released values so
  cff-convert validation passes; leave commented hints for when
  Zenodo mints a real DOI.
- docs/conf.py: shorten the nbsphinx comment to fit the new ruff
  line-length, and document why html_logo and the README image point
  at different files in _static/.
@mahynski
mahynski merged commit 5a8fdc0 into main May 3, 2026
8 of 9 checks passed
@mahynski
mahynski deleted the claude/code-review-XO0JW branch May 3, 2026 11:20
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.

2 participants