Skip to content

fix: show "No ADRs found" fallback in adr-list#136

Open
SecKatie wants to merge 2 commits into
mainfrom
fix/adr-list-empty-output
Open

fix: show "No ADRs found" fallback in adr-list#136
SecKatie wants to merge 2 commits into
mainfrom
fix/adr-list-empty-output

Conversation

@SecKatie
Copy link
Copy Markdown
Collaborator

@SecKatie SecKatie commented May 18, 2026

Summary

  • make adr-list was silent when docs/adr/ contained no numbered ADRs (only README.md and template.md). Neither the list nor the "No ADRs found" fallback printed.
  • Root cause: the recipe relied on ls | grep | sort pipeline exit codes. When grep matched nothing it exited 1, but sort of empty input exits 0, so the pipeline succeeded and the || fallback never fired. (Shell precedence also tied the || to the whole pipeline rather than to the cd.)
  • Fix: capture the filtered list into a variable and branch on whether it's empty.

Fixes #135

Test plan

  • make adr-list with only README.md + template.md present → prints "No ADRs found…"
  • make adr-list with numbered ADRs present → prints sorted list
  • make adr-list with docs/adr/ missing → prints "No ADRs found…"

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Chores
    • Enhanced error handling and user feedback when listing Architecture Decision Records, now gracefully handles missing or empty ADR directories with helpful guidance for creating new records.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a96d4541-eae7-4d30-a30f-a42f88976366

📥 Commits

Reviewing files that changed from the base of the PR and between a8785f5 and 5931027.

📒 Files selected for processing (1)
  • mk/adr.mk

📝 Walkthrough

Walkthrough

The adr-list make target now captures sorted numeric-prefixed ADR markdown files into a variable and uses conditional output to display the list or suggest creating a new ADR, fixing a bug where no output appeared when the ADR directory contained no matching files.

Changes

ADR List Error Handling

Layer / File(s) Summary
ADR list robustness
mk/adr.mk
The adr-list target now assigns sorted numeric-prefixed ADR files to a found variable with suppressed cd/ls errors, then conditionally prints the list or a "No ADRs found" message with usage hint, replacing a pipeline where sort on empty input masked the grep failure code.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A makefile's wisdom, once obscured,
Now glimmers clear with logic assured.
Empty pipes no longer hide the way—
The rabbit's fix brings truth to day! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: show "No ADRs found" fallback in adr-list' directly describes the main change: fixing the display of a fallback message when no ADRs exist, which is the core objective of the PR.
Linked Issues check ✅ Passed The PR successfully addresses issue #135 by implementing the proposed fix: capturing the filtered ADR list into a variable and branching on emptiness to properly show the 'No ADRs found' fallback when no numbered ADRs exist.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the adr-list target in mk/adr.mk to address the fallback message issue described in #135; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/adr-list-empty-output

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator Author

@SecKatie SecKatie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work and the code looks good to me.

Comment thread mk/adr.mk Outdated
@SecKatie SecKatie force-pushed the fix/adr-list-empty-output branch from 1323fc8 to 9fedc01 Compare May 19, 2026 15:43
SecKatie and others added 2 commits May 19, 2026 18:58
…exist

The previous recipe relied on a pipeline of `ls | grep | sort` to decide
whether to fall back. When grep matched nothing it exited 1, but `sort`
of empty input exits 0, so the pipeline succeeded and the `||` branch
never fired — leaving `make adr-list` silent. Capture the filtered list
into a variable and branch on whether it's empty instead.

Fixes #135

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@SecKatie SecKatie force-pushed the fix/adr-list-empty-output branch from 9fedc01 to 5931027 Compare May 19, 2026 22:58
@mrbrandao
Copy link
Copy Markdown
Collaborator

mrbrandao commented May 25, 2026

I'm planning to drop adr-tools and simplify ADRs to be pure names. I have it proposed in the PR #109

Let's place these changes here on hold, we'll rejoin when we have it set from PR 109

@mrbrandao mrbrandao added the wontfix This will not be worked on label May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make adr-list shows no output when no numbered ADRs exist

3 participants