PR #1046 follow-ups: stamp-failure UX, dead REVIEWER constant nf-od2 ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/1053 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #1046

  1. The Next: ./tools/pr/review_now.py <n> hint disappears whenever the (now routinely-fallible) approval:* stamp fails. tools/nflib/devcli.py verb_pr: step()'s fatal=False (added by #1046) still appends to the shared failures list, and the closing block

```python if failures: print(...) return code print(f"\nNext: ./tools/pr/review_now.py {number} ...") return 0 ```

returns early on ANY non-empty failures, fatal or not — so a stamp failure (exit code still 0, by design) silently drops the standard 'pull the review' guidance too. Low real-world impact (dev.py status still surfaces the same next action), but it undercuts the PR's own stated intent that an informational failure 'must not' disrupt the dev's flow. Fix: separate hard failures from informational ones (e.g. only early-return on a failure with fatal=True), or just always print the Next: line.

  1. REVIEWER = "aedanpope" (tools/nflib/devcli.py:127) is now dead. #1046 deleted its only call site (the review-request step) and nothing else in the repo imports devcli.REVIEWER. It's still exported via __all__ with a comment justifying it as 'constants rather than flags.' DEV_PRINCIPLES B12: relish deleting code — worth removing along with the now-stale justification comment, or confirming there's a reason to keep it as public API.

Dependencies

None.

Comments

No comments.

Add a comment