Skip to content

Conversation

@michalskrivanek
Copy link
Contributor

@michalskrivanek michalskrivanek commented Oct 11, 2025

for jumpstarter-dev/jumpstarter-controller#168

Summary by CodeRabbit

  • New Features
    • Lease duration is now optional, allowing creation or update of leases without specifying a duration. Systems may apply sensible defaults where omitted.
    • Improves flexibility and compatibility for clients that cannot or do not provide a duration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 11, 2025

Walkthrough

The Lease message in proto/jumpstarter/client/v1/client.proto updates the duration field: it removes the REQUIRED google.api.field_behavior option and marks the field as optional, changing presence semantics without other structural modifications.

Changes

Cohort / File(s) Summary of Changes
Lease duration presence change
proto/jumpstarter/client/v1/client.proto
In message Lease, field duration changed from google.protobuf.Duration duration = 3 [(google.api.field_behavior) = REQUIRED]; to optional google.protobuf.Duration duration = 3;, removing the REQUIRED annotation and enabling field presence.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • mangelajo

Poem

A hop and a nibble, I tweak with delight,
Duration now optional—less rigid, more light.
Carrots compile, protobufs align,
Fields wave hello with presence divine.
Thump-thump the build, green lights in sight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title "endtime" is vague and does not summarize the primary change, which involves making the Lease.duration field optional in the client.proto file. Because it does not relate to or convey the key modification, it fails to accurately describe the pull request. Please update the title to clearly reflect the change to the Lease.duration field, for example “Make Lease.duration optional in proto/jumpstarter/client/v1/client.proto.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e34d86a and 8cccfa6.

📒 Files selected for processing (1)
  • proto/jumpstarter/client/v1/client.proto (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • proto/jumpstarter/client/v1/client.proto

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

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f8035c and e34d86a.

📒 Files selected for processing (2)
  • proto/jumpstarter/client/v1/client.proto (1 hunks)
  • proto/jumpstarter/v1/jumpstarter.proto (1 hunks)

It could be calculated when EndTime is provided.
Logic is validated in controller.
@mangelajo mangelajo merged commit f67bf42 into jumpstarter-dev:main Oct 13, 2025
2 checks passed
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.

2 participants