Skip to content

chore(base-cluster): change value name to camel#1777

Merged
SyeKlu merged 1 commit intomainfrom
fix-value-typo
Oct 31, 2025
Merged

chore(base-cluster): change value name to camel#1777
SyeKlu merged 1 commit intomainfrom
fix-value-typo

Conversation

@SyeKlu
Copy link
Copy Markdown
Contributor

@SyeKlu SyeKlu commented Oct 31, 2025

Summary by CodeRabbit

  • Chores
    • Updated Loki configuration property naming in Helm chart to follow naming conventions (retention_period → retentionPeriod). Update custom configurations if using Loki retention settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 31, 2025

Walkthrough

The PR renames the Loki retention configuration property from snake_case (retention_period) to camelCase (retentionPeriod) across the Helm chart template, schema definition, and values file, maintaining the same 45-day retention value.

Changes

Cohort / File(s) Summary
Loki Retention Configuration Renaming
charts/base-cluster/templates/monitoring/logs/loki.yaml, charts/base-cluster/values.schema.json, charts/base-cluster/values.yaml
Renamed Loki retention property from retention_period to retentionPeriod across template, schema, and values files; retention value remains 45d

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Verify naming consistency applied uniformly across all three files
  • Confirm the schema definition aligns with both template and values usage

Possibly related PRs

  • #1774 — Introduces the retention_period property in Loki configuration that this PR renames to retentionPeriod
  • #1745 — Modifies the same limits_config.retention_period Loki setting that this PR refactors

Suggested labels

base-cluster

Suggested reviewers

  • tasches
  • cwrau
  • marvinWolff
  • teutonet-bot

Poem

🐰 From snake_case hops to camelCase bounds,
Loki's retention takes cleaner grounds,
Forty-five days in a prettier dress,
Configuration refactored—hoppy success! ✨

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "chore(base-cluster): change value name to camel" directly aligns with the main changes in the PR. All three modified files (loki.yaml, values.schema.json, and values.yaml) are consistently renaming the retention_period property to retentionPeriod, converting from snake_case to camelCase. The title is specific and descriptive about the type of change (naming convention conversion) while remaining concise and clear. It avoids vague terms and would allow a teammate scanning commit history to understand the nature of the change without detailed inspection.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-value-typo

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
Copy Markdown

@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: 0

🧹 Nitpick comments (1)
charts/base-cluster/values.schema.json (1)

315-315: Consider communicating breaking change to users.

While the rename itself is straightforward and consistent, this constitutes a breaking change for users with custom values files using the old retention_period key. Consider ensuring that release notes include migration guidance (e.g., retention_periodretentionPeriod in user values).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb46a72 and 72432ec.

📒 Files selected for processing (3)
  • charts/base-cluster/templates/monitoring/logs/loki.yaml (1 hunks)
  • charts/base-cluster/values.schema.json (1 hunks)
  • charts/base-cluster/values.yaml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: check licenses
  • GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (3)
charts/base-cluster/values.yaml (1)

315-315: Clean rename to align with camelCase convention.

The property rename from retention_period to retentionPeriod is consistent with Helm chart naming conventions and maintains the value correctly.

charts/base-cluster/templates/monitoring/logs/loki.yaml (1)

81-81: Correct reference to renamed property.

The template correctly references the renamed value. The retention_period key on the left is Loki's native configuration (intentionally snake_case), while retentionPeriod on the right is our Helm value naming.

charts/base-cluster/values.schema.json (1)

924-928: Schema properly updated with clear validation and examples.

The property rename is reflected in the schema with appropriate pattern validation and helpful examples. This maintains consistency with the values.yaml change.

@SyeKlu SyeKlu added this pull request to the merge queue Oct 31, 2025
Merged via the queue into main with commit b3bd6be Oct 31, 2025
32 of 33 checks passed
@SyeKlu SyeKlu deleted the fix-value-typo branch October 31, 2025 10:29
github-merge-queue Bot pushed a commit that referenced this pull request Oct 31, 2025
🤖 I have created a release *beep* *boop*
---


##
[10.0.3](base-cluster-v10.0.2...base-cluster-v10.0.3)
(2025-10-31)


### Miscellaneous Chores

* **base-cluster:** change value name to camel
([#1777](#1777))
([b3bd6be](b3bd6be))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
  * Released version 10.0.3 of base-cluster
  * Updated configuration value naming to camel case convention

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants