Documentation: WARNING — source files changed but no docs updated
Tests: WARNING — source files changed but no test files updated
Commit conventions: OK
No issues found by automated checks.
Auto-merge: If all CI checks pass, this PR will be merged automatically.
## Automated PR Review
- Architecture compliance: OK
- Best practices: OK
- Security: OK
- i18n: OK
- Resource management: OK
- Documentation: WARNING — source files changed but no docs updated
- Tests: WARNING — source files changed but no test files updated
- Commit conventions: OK
No issues found by automated checks.
---
**Auto-merge:** If all CI checks pass, this PR will be merged automatically.
kireto
approved these changes 2026-07-07 12:03:47 +00:00
Manual review of PR #183 — fix: preserve .badges/ dir during git clean in push_badges.
Diff: 1 file, 2 insertions, 2 deletions in src/devx/ci/push_badges.py.
The change adds -e badges_dir to the git clean -fdx invocation in push_to_badges_branch() so generated SVGs survive until they are copied
to the repo root, fixing the exit-128 failure on git add ./*.svg.
Per-category findings (13/13 confirmed):
Architecture — No subprocess in CLI, no hardcoded URLs, proper module
separation. The fix is localized to the existing CI module; no new
imports or cross-module coupling introduced.
Code quality — No print(), no bare except, no TODO/FIXME. Touched
function remains well under 50 lines. Comment updated to reflect the
new behavior.
Security — No hardcoded secrets, no shell=True (subprocess.run uses
list form with check=True), no eval/exec, no secrets in logs. The # nosec B607 annotation is preserved and still appropriate.
i18n — No user-facing strings changed by this diff; existing _()
wrappers in the surrounding code are untouched.
Testing — Existing test suite (tests/unit/test_push_badges.py) covers
push_to_badges_branch with mocked subprocess. make pytest-cov passes
with 100% coverage (1735 tests, 7.37s). Tests are fast (<0.5s each).
Performance — No new loops or I/O. The fix adds one CLI flag to an
existing git invocation; no measurable overhead.
UX — Error path that produced the raw exit-128 traceback is now
prevented; users get the normal "Badges pushed to badges branch"
success message instead.
Documentation — Inline comment updated to explain why .badges/ is
preserved. No README/AGENTS.md change needed for a one-line bugfix in
internal CI logic.
Workflow — Branch DEVX-120-fix-badges-push matches task prefix DEVX-120. Single commit fix: preserve .badges/ dir during git clean in push_badges uses conventional commit format with no task-ID
prefix. No merge commits. Vikunja task DEVX-120 exists ("Fix badges
push: git clean removes .badges/ before copy") and matches the PR
subject.
Maintainability — No dead code, no unused imports, no commented-out
code. The added -e badges_dir reuses the existing function
parameter, keeping the exclusion in sync with the copy target.
Resource management — subprocess.run uses check=True (via _run
helper); no open()/Popen without cleanup. No resource leaks
introduced.
Backwards compatibility — No public API change. The default --output-dir .badges/ continues to work; the new exclusion uses
the same parameter value, so behavior is identical for existing
callers (only difference: .badges/ is no longer deleted prematurely).
Logging — No sensitive data logged. Log messages unchanged; proper
info-level echo() calls retained. No excessive logging.
Quality gates: make pytest-cov → 100% coverage, 1735 passed. make lint-all → ruff + pyright + bandit + actionlint + hadolint all
pass with no issues.
Verdict: APPROVE.
Manual review of PR #183 — fix: preserve .badges/ dir during git clean in push_badges.
Diff: 1 file, 2 insertions, 2 deletions in src/devx/ci/push_badges.py.
The change adds `-e badges_dir` to the `git clean -fdx` invocation in
`push_to_badges_branch()` so generated SVGs survive until they are copied
to the repo root, fixing the exit-128 failure on `git add ./*.svg`.
Per-category findings (13/13 confirmed):
1. Architecture — No subprocess in CLI, no hardcoded URLs, proper module
separation. The fix is localized to the existing CI module; no new
imports or cross-module coupling introduced.
2. Code quality — No print(), no bare except, no TODO/FIXME. Touched
function remains well under 50 lines. Comment updated to reflect the
new behavior.
3. Security — No hardcoded secrets, no shell=True (subprocess.run uses
list form with check=True), no eval/exec, no secrets in logs. The
`# nosec B607` annotation is preserved and still appropriate.
4. i18n — No user-facing strings changed by this diff; existing _()
wrappers in the surrounding code are untouched.
5. Testing — Existing test suite (tests/unit/test_push_badges.py) covers
push_to_badges_branch with mocked subprocess. `make pytest-cov` passes
with 100% coverage (1735 tests, 7.37s). Tests are fast (<0.5s each).
6. Performance — No new loops or I/O. The fix adds one CLI flag to an
existing git invocation; no measurable overhead.
7. UX — Error path that produced the raw exit-128 traceback is now
prevented; users get the normal "Badges pushed to badges branch"
success message instead.
8. Documentation — Inline comment updated to explain why .badges/ is
preserved. No README/AGENTS.md change needed for a one-line bugfix in
internal CI logic.
9. Workflow — Branch `DEVX-120-fix-badges-push` matches task prefix
`DEVX-120`. Single commit `fix: preserve .badges/ dir during git clean
in push_badges` uses conventional commit format with no task-ID
prefix. No merge commits. Vikunja task DEVX-120 exists ("Fix badges
push: git clean removes .badges/ before copy") and matches the PR
subject.
10. Maintainability — No dead code, no unused imports, no commented-out
code. The added `-e badges_dir` reuses the existing function
parameter, keeping the exclusion in sync with the copy target.
11. Resource management — subprocess.run uses check=True (via _run
helper); no open()/Popen without cleanup. No resource leaks
introduced.
12. Backwards compatibility — No public API change. The default
`--output-dir .badges/` continues to work; the new exclusion uses
the same parameter value, so behavior is identical for existing
callers (only difference: .badges/ is no longer deleted prematurely).
13. Logging — No sensitive data logged. Log messages unchanged; proper
info-level echo() calls retained. No excessive logging.
Quality gates: `make pytest-cov` → 100% coverage, 1735 passed.
`make lint-all` → ruff + pyright + bandit + actionlint + hadolint all
pass with no issues.
Verdict: APPROVE.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
kireto
emil
Automated PR Review
No issues found by automated checks.
Auto-merge: If all CI checks pass, this PR will be merged automatically.
Auto-approved: all CI checks passed (quality, pr-review, release-dry-run).
Manual review of PR #183 — fix: preserve .badges/ dir during git clean in push_badges.
Diff: 1 file, 2 insertions, 2 deletions in src/devx/ci/push_badges.py.
The change adds
-e badges_dirto thegit clean -fdxinvocation inpush_to_badges_branch()so generated SVGs survive until they are copiedto the repo root, fixing the exit-128 failure on
git add ./*.svg.Per-category findings (13/13 confirmed):
separation. The fix is localized to the existing CI module; no new
imports or cross-module coupling introduced.
function remains well under 50 lines. Comment updated to reflect the
new behavior.
list form with check=True), no eval/exec, no secrets in logs. The
# nosec B607annotation is preserved and still appropriate.wrappers in the surrounding code are untouched.
push_to_badges_branch with mocked subprocess.
make pytest-covpasseswith 100% coverage (1735 tests, 7.37s). Tests are fast (<0.5s each).
existing git invocation; no measurable overhead.
prevented; users get the normal "Badges pushed to badges branch"
success message instead.
preserved. No README/AGENTS.md change needed for a one-line bugfix in
internal CI logic.
DEVX-120-fix-badges-pushmatches task prefixDEVX-120. Single commitfix: preserve .badges/ dir during git clean in push_badgesuses conventional commit format with no task-IDprefix. No merge commits. Vikunja task DEVX-120 exists ("Fix badges
push: git clean removes .badges/ before copy") and matches the PR
subject.
code. The added
-e badges_dirreuses the existing functionparameter, keeping the exclusion in sync with the copy target.
helper); no open()/Popen without cleanup. No resource leaks
introduced.
--output-dir .badges/continues to work; the new exclusion usesthe same parameter value, so behavior is identical for existing
callers (only difference: .badges/ is no longer deleted prematurely).
info-level echo() calls retained. No excessive logging.
Quality gates:
make pytest-cov→ 100% coverage, 1735 passed.make lint-all→ ruff + pyright + bandit + actionlint + hadolint allpass with no issues.
Verdict: APPROVE.