Skip to content

fix: credit then invoice config name#4398

Merged
turip merged 1 commit into
mainfrom
chore/fix-credit-then-invoice
May 20, 2026
Merged

fix: credit then invoice config name#4398
turip merged 1 commit into
mainfrom
chore/fix-credit-then-invoice

Conversation

@turip

@turip turip commented May 20, 2026

Copy link
Copy Markdown
Member

Overview

We use camelCase for these variables.

Notes for reviewer

Summary by CodeRabbit

  • Chores
    • Updated billing configuration naming convention: the credits setting has been renamed from enable_credit_then_invoice to enableCreditThenInvoice. Users must update existing configuration files to use the new key name.

Review Change Stack

@turip turip requested a review from a team as a code owner May 20, 2026 14:15
@turip turip added release-note/bug-fix Release note: Bug Fixes area/billing labels May 20, 2026
@turip turip enabled auto-merge (squash) May 20, 2026 14:15
@turip turip requested a review from borbelyr-kong May 20, 2026 14:15
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR standardizes the configuration field naming for the credit-then-invoice feature from snake_case to camelCase. The struct tag, default configuration key, example config, and e2e test configuration are all updated consistently to use enableCreditThenInvoice.

Changes

Credit Configuration Naming

Layer / File(s) Summary
Configuration key naming update
app/config/credits.go, config.example.yaml, e2e/config.yaml
The EnableCreditThenInvoice field YAML tag is updated to enableCreditThenInvoice, the default Viper key registration is updated to match, and both example and e2e configurations are renamed consistently.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • openmeterio/openmeter#4136: Introduced the enable_credit_then_invoice feature flag that this PR renames to follow camelCase convention.

Suggested reviewers

  • GAlexIHU
  • tothandras
🚥 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%. 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 accurately summarizes the main change: renaming the credit configuration key from snake_case to camelCase.
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/fix-credit-then-invoice

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/config/credits.go`:
- Line 11: The new YAML key change breaks backward compatibility; update the
config parsing so both the new field EnableCreditThenInvoice
(yaml:"enableCreditThenInvoice") and the legacy key enable_credit_then_invoice
are supported during migration: either add a second struct field (e.g.,
LegacyEnableCreditThenInvoice bool `yaml:"enable_credit_then_invoice"`) and
coalesce its value into EnableCreditThenInvoice after unmarshalling, or
implement a custom UnmarshalYAML for the config struct that reads both keys and
sets EnableCreditThenInvoice when the legacy key is present; ensure precedence
rules (explicit new key wins) and that default behavior remains unchanged when
neither key is present.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ab49f75c-7e69-4f21-b6c0-1cdd16aa558b

📥 Commits

Reviewing files that changed from the base of the PR and between 9700167 and 297a5e8.

📒 Files selected for processing (3)
  • app/config/credits.go
  • config.example.yaml
  • e2e/config.yaml

Comment thread app/config/credits.go
type CreditsConfiguration struct {
Enabled bool `yaml:"enabled"`
EnableCreditThenInvoice bool `yaml:"enable_credit_then_invoice"`
EnableCreditThenInvoice bool `yaml:"enableCreditThenInvoice"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Please keep backward compatibility for the legacy key during migration.

At Line 11 and Line 24, switching only to enableCreditThenInvoice can break existing configs still using enable_credit_then_invoice and silently change behavior (defaulting to false). Could we support both keys for at least one transition window (or add explicit migration handling)?

Also applies to: 24-24

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/config/credits.go` at line 11, The new YAML key change breaks backward
compatibility; update the config parsing so both the new field
EnableCreditThenInvoice (yaml:"enableCreditThenInvoice") and the legacy key
enable_credit_then_invoice are supported during migration: either add a second
struct field (e.g., LegacyEnableCreditThenInvoice bool
`yaml:"enable_credit_then_invoice"`) and coalesce its value into
EnableCreditThenInvoice after unmarshalling, or implement a custom UnmarshalYAML
for the config struct that reads both keys and sets EnableCreditThenInvoice when
the legacy key is present; ensure precedence rules (explicit new key wins) and
that default behavior remains unchanged when neither key is present.

@turip turip merged commit 944c12d into main May 20, 2026
30 of 32 checks passed
@turip turip deleted the chore/fix-credit-then-invoice branch May 20, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants