flag: make Apply return an error and drop errorableFlag

[taran-p]

address v2 TODO.

Summary by CodeRabbit

• Bug Fixes

  • Flag registration errors are now returned and handled consistently.
  • Improved error reporting when flags cannot be applied.

• Refactor

  • Streamlined flag application across supported flag types.
  • Standardized flag registration behavior through a single error-aware interface.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: db3428a3-fa4b-43af-bc00-853470aef15c

📥 Commits

Reviewing files that changed from the base of the PR and between 76837d2 and 0cf0cf5.

📒 Files selected for processing (2)

• app_test.go
• flag.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The flag API now returns errors directly from Apply. Concrete flag types perform registration and environment parsing in Apply. The obsolete errorableFlag interface and ApplyWithError methods were removed.

Changes

Flag application API

Layer / File(s)	Summary
Unify flag application errors flag.go	Flag.Apply now returns an error. flagSet calls Apply directly and returns its error.
Update flag implementations flag.go, app_test.go	All flag types perform parsing and registration in Apply and return errors. The test flag helper now returns nil.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 0cf0c

This change makes flag application errors propagate before command execution, improving failure handling, but it also requires downstream custom flag implementations to update their method signature before dependent applications can rebuild. The PR is mergeable with explicit owner awareness of that migration requirement.

Poem

I’m a rabbit with flags in a row
Each Apply now tells errors where to go
Old helpers hop away
New returns guide the way
And registrations bloom in the flow

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main API change: making Apply return an error and removing errorableFlag.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}