Skip to content

remove reloader annotations#75

Merged
agaetep merged 1 commit intomainfrom
agaete/reloader
Oct 30, 2025
Merged

remove reloader annotations#75
agaetep merged 1 commit intomainfrom
agaete/reloader

Conversation

@agaetep
Copy link
Contributor

@agaetep agaetep commented Oct 30, 2025

These annotations are needed in argo, not under this repo.

Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings October 30, 2025 17:22
@agaetep agaetep requested review from a team and emsearcy as code owners October 30, 2025 17:22
@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

This PR updates the Helm chart version from 0.3.4 to 0.3.5 and removes Reloader auto-restart annotations (reloader.stakater.com/auto: "true") from the openfga, heimdall, and lfx-v2-meeting-service configurations.

Changes

Cohort / File(s) Change Summary
Chart Metadata
charts/lfx-platform/Chart.yaml
Version bumped from 0.3.4 to 0.3.5
Reloader Annotations Removal
charts/lfx-platform/values.yaml
Removed reloader.stakater.com/auto: "true" annotation blocks from openfga, heimdall, and lfx-v2-meeting-service sections, disabling automatic pod restarts on ConfigMap/Secret changes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Annotation removals are repetitive across three similar service sections with consistent patterns
  • Version bump is straightforward metadata change
  • No logic modifications or control-flow alterations

Possibly related PRs

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
Title Check ✅ Passed The pull request title "remove reloader annotations" directly and clearly describes the main substantive change in the changeset: the removal of Reloader annotation configurations (reloader.stakater.com/auto: "true") from multiple sections in the values.yaml file. The title is specific, concise, and conveys the primary change without unnecessary noise. While the pull request also includes a version bump in Chart.yaml (0.3.4 to 0.3.5), this is a secondary administrative change, and the title appropriately focuses on the main point of the changeset.
Description Check ✅ Passed The pull request description "These annotations are needed in argo, not under this repo." is related to the changeset and provides reasonable context for why the reloader annotations are being removed. The description connects to the main change by explaining that these annotations belong elsewhere (in Argo configuration) rather than in this repository. While brief, it is not vague or completely off-topic, and it meaningfully explains the rationale behind the removal.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch agaete/reloader

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a69c0af and bbeec59.

📒 Files selected for processing (2)
  • charts/lfx-platform/Chart.yaml (1 hunks)
  • charts/lfx-platform/values.yaml (0 hunks)
💤 Files with no reviewable changes (1)
  • charts/lfx-platform/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
📚 Learning: 2025-08-29T16:53:12.710Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).

Applied to files:

  • charts/lfx-platform/Chart.yaml
🔇 Additional comments (1)
charts/lfx-platform/Chart.yaml (1)

8-8: Verify version bump is consistent with maintainer's versioning preferences.

The patch version increment (0.3.4 → 0.3.5) is appropriate for removing optional annotations. However, based on previous feedback, the maintainer has shown a preference for conservative version bumping—choosing 0.2.7 over 0.3.0 even for behavior-altering changes. Please confirm that this version increment aligns with the maintainer's versioning strategy and that it's justified solely by the removal of Reloader annotations.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes redundant Reloader annotations from multiple service configurations in the Helm chart, cleaning up the values file by eliminating duplicate configuration blocks.

  • Removed Reloader annotations from openfga, heimdall, and lfx-v2-meeting-service configurations
  • Bumped chart version from 0.3.4 to 0.3.5

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
charts/lfx-platform/values.yaml Removed redundant reloader.stakater.com/auto annotation blocks from three service configurations
charts/lfx-platform/Chart.yaml Incremented chart version to reflect the cleanup changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@agaetep agaetep merged commit a056f68 into main Oct 30, 2025
10 checks passed
@agaetep agaetep deleted the agaete/reloader branch October 30, 2025 22:17
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.

3 participants