Skip to content

fix: add optional to default_value#25

Merged
yordis merged 1 commit intomainfrom
fix-add-optional
Jan 31, 2026
Merged

fix: add optional to default_value#25
yordis merged 1 commit intomainfrom
fix-add-optional

Conversation

@yordis
Copy link
Member

@yordis yordis commented Jan 31, 2026

Signed-off-by: Yordis Prieto yordis.prieto@gmail.com

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor
Copy link

cursor bot commented Jan 31, 2026

PR Summary

Medium Risk
Proto schema change may affect generated code and any consumers that assumed default_value was always present or used empty string as the required/optional signal.

Overview
Clarifies required-vs-optional semantics for environment-backed config fields by changing EnvVarOption.default_value from string to optional string, so generators can differentiate unset (required) from set to "" (optional with empty default).

Updates the inline schema/docs examples and invariants in options.proto to reflect the new behavior and show optional empty-string defaults and required repeated fields.

Written by Cursor Bugbot for commit e94258e. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

Walkthrough

A protobuf message field in the EnvVarOption message is modified from a required string to an optional string, with accompanying documentation updates clarifying that an unset default_value indicates a required field, while any set value makes the field optional with that default.

Changes

Cohort / File(s) Summary
Protobuf Schema Updates
proto/trogon/env/v1alpha1/options.proto
Modified EnvVarOption.default_value field from string to optional string, enabling presence semantics. Documentation expanded to clarify required vs optional field behavior based on whether default_value is set.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A field once strict, now holds the choice,
Default or not—the schema's voice!
Optional now, with presence so clear,
Documentation blooms, no questions here! 🌱✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description only contains a sign-off line and lacks meaningful information about the changes, but it is not explicitly unrelated to the changeset. Provide a more detailed description explaining why the optional keyword was added, what impact this has on the API contract, and any breaking changes or migration notes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding the 'optional' keyword to the default_value field in the proto file, which matches the primary modification in the changeset.
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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-add-optional

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.

@github-actions
Copy link

github-actions bot commented Jan 31, 2026

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed⏩ skippedJan 31, 2026, 3:19 AM

@yordis yordis marked this pull request as ready for review January 31, 2026 03:19
@yordis yordis merged commit c90925d into main Jan 31, 2026
5 of 6 checks passed
@yordis yordis deleted the fix-add-optional branch January 31, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant