Skip to content

fix: --confirm=false wasn't working#105

Open
sylr wants to merge 1 commit intomainfrom
fix/confirm-flag
Open

fix: --confirm=false wasn't working#105
sylr wants to merge 1 commit intomainfrom
fix/confirm-flag

Conversation

@sylr
Copy link
Contributor

@sylr sylr commented Jan 23, 2026

No description provided.

@sylr sylr requested a review from a team as a code owner January 23, 2026 09:04
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Walkthrough

NeedConfirm() now returns the confirm flag value directly and WithConfirmFlag() sets that flag's default to true; checks in approval functions were adjusted to use the new NeedConfirm() behavior, altering when interactive confirmation is skipped versus prompted.

Changes

Cohort / File(s) Summary
Approval logic
pkg/approval.go
NeedConfirm(cmd) changed to return the confirmFlag value directly; callers updated to use if !NeedConfirm(cmd) for early return, changing control flow for prompting.
Flag default
pkg/command.go
WithConfirmFlag() now registers the confirm boolean flag with default true instead of false, flipping the flag's default behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I twitch my nose at flipped routine,
A flag now true where false had been.
Skip the prompt when lights are green,
Hop through logic, soft and clean. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset. Add a description explaining the issue, the fix applied, and its impact on users to provide context for reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: --confirm=false wasn't working' directly addresses the main change: altering the default value of the --confirm flag from false to true to fix the behavior when --confirm=false is used.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/confirm-flag

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
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@pkg/approval.go`:
- Around line 31-34: CheckOrganizationApprobation has inverted confirm-flag
logic compared to CheckStackApprobation: when the flag is true it currently
skips the prompt. Update CheckOrganizationApprobation to match
CheckStackApprobation by negating the check (e.g., use if !GetBool(cmd,
confirmFlag) { /* show prompt */ } or reuse NeedConfirm/GetBool consistently) so
that --confirm=true shows the interactive prompt and --confirm=false skips it;
locate and change the conditional in the CheckOrganizationApprobation function
accordingly.

Signed-off-by: Sylvain Rabot <sylvain@formance.com>
@sylr sylr force-pushed the fix/confirm-flag branch from c5652a5 to b12c06b Compare January 23, 2026 09:41
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.

1 participant