Skip to content

Conversation

mikeshng
Copy link
Member

@mikeshng mikeshng commented Oct 12, 2025

Summary

fix: ManifestWorkReplicaSet applied capitalization error

Related issue(s)

Found this weird capitalization while working on open-cluster-management-io/ocm#1054

Summary by CodeRabbit

  • Refactor

    • Renamed status summary field from Applied to applied across the ManifestWorkReplicaSet API, including top-level summary and placementSummary sections, aligning JSON/YAML output.
  • Bug Fixes

    • Updated CRD schema to validate the new summary.applied field consistently.
  • Impact

    • Clients referencing status.summary.Applied must switch to status.summary.applied (including placementSummary). Update manifests, scripts, and dashboards to reflect the new field name.

@openshift-ci openshift-ci bot requested review from deads2k and mdelder October 12, 2025 18:13
@mikeshng
Copy link
Member Author

/assign @qiujian16

Copy link

coderabbitai bot commented Oct 12, 2025

Walkthrough

Renames the summary field key from Applied to applied in the ManifestWorkReplicaSet CRD schema and aligns the Go struct’s JSON tag accordingly. No type changes or logic modifications are introduced.

Changes

Cohort / File(s) Summary of Changes
CRD schema field key rename
work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml
In status schema, renamed summary.Applied to summary.applied in both status/placementSummary.properties and status/summary.properties.
Go type JSON tag alignment
work/v1alpha1/types_manifestworkreplicaset.go
Updated ManifestWorkReplicaSetSummary.Applied struct tag from json:"Applied" to json:"applied".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly highlights the main change—correcting the capitalization of the ManifestWorkReplicaSet applied field—uses the appropriate bug fix prefix and emoji, and is clear and specific without extraneous details.
Description Check ✅ Passed The description follows the repository template’s required sections by providing both a clear Summary and a Related issue(s) section with a link to the relevant issue, adequately explaining the intent of the change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

🧹 Nitpick comments (1)
work/v1alpha1/types_manifestworkreplicaset.go (1)

135-135: Approve JSON tag change; update docs and schemas
The JSON tag change to applied follows Kubernetes conventions and no references to the old Applied tag remain. This is a breaking change—update CRD schemas, examples, and user documentation to reflect the new field name.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48b7234 and 29dd31c.

📒 Files selected for processing (2)
  • work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml (2 hunks)
  • work/v1alpha1/types_manifestworkreplicaset.go (1 hunks)
⏰ 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). (1)
  • GitHub Check: verify
🔇 Additional comments (1)
work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml (1)

744-744: LGTM! Both occurrences updated consistently.

The CRD schema correctly renames the field from Applied to applied in both locations:

  • Line 744: status.placementSummary[].summary.applied
  • Line 766: status.summary.applied

The field descriptions are preserved appropriately, and the changes align with the Go struct JSON tag update in types_manifestworkreplicaset.go.

Also applies to: 766-766

@qiujian16
Copy link
Member

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Oct 13, 2025
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikeshng, qiujian16

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 87a44af into open-cluster-management-io:main Oct 13, 2025
12 checks passed
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.

2 participants