Skip to content

Conversation

@maximiliantech
Copy link
Member

@maximiliantech maximiliantech commented Jun 2, 2025

This pull request introduces significant updates to the Makefile, PROJECT, and README.md files to enhance the build process, streamline resource definitions, and improve documentation. Key changes include the restructuring of the Makefile for better organization and functionality, updates to resource paths in the PROJECT file, and expanded documentation in the README.md to provide more clarity and usability.

Build System Enhancements:

  • Improved Makefile structure: Added new sections for build dependencies, testing, deployment, and local development utilities. This includes targets for downloading tools like kustomize, golangci-lint, and controller-gen, as well as new commands for creating and managing local kind clusters. [1] [2] [3]
  • Enhanced linting and testing: Introduced lint and lint-fix targets for code quality checks and automated fixes, and restructured the test target for better integration with environment setup. [1] [2]

Resource Configuration Updates:

  • Updated resource paths in PROJECT: Migrated several resources from v1beta1 to v1alpha1 for consistency and alignment with the latest API definitions. This affects resources like ClusterAccess, FederatedMetric, and FederatedManagedMetric.

Documentation Improvements:

  • Expanded README.md: Added a detailed table of contents, resource type descriptions with links to CRD files, and sections on data sink integration and migration from legacy configurations. This improves accessibility and clarity for users and contributors. [1] [2]

These changes collectively improve the maintainability, usability, and developer experience of the project.

@maximiliantech maximiliantech changed the title chore: clean-up and improve repo setup chore: clean-up and improve repository setup Jun 2, 2025
@maximiliantech maximiliantech requested a review from Copilot June 3, 2025 14:29
Copy link

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 standardizes the operator name, removes legacy v1beta1 CRDs, and updates controller setup calls.

  • Renamed co-metrics-operator labels and image references to metrics-operator across RBAC, Prometheus, manager, and default configs
  • Deleted obsolete clusteraccesses v1beta1 CRDs in config/crd and embedded directory
  • Replaced setupReconcilersV1beta1 with setupFederatedMetricController and setupFederatedManagedMetricController in cmd/main.go
  • Updated Helm chart repository paths and example names in README

Reviewed Changes

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

Show a summary per file
File Description
config/rbac/*.yaml Renamed labels co-metrics-operatormetrics-operator
config/crd/bases/metrics.cloud.sap_clusteraccesses.yaml Removed obsolete CRD v1beta1
cmd/main.go Dropped v1beta1 setup, added federated controllers
charts/metrics-operator/values.yaml.tpl & values.yaml Updated image repository URL
README.md Simplified example resource names
PROJECT Cleaned up deprecated API entries
Comments suppressed due to low confidence (3)

charts/metrics-operator/values.yaml.tpl:8

  • The image repository path includes github.com in the URL, which looks unusual for a container registry. Please confirm the correct registry path (e.g., ghcr.io/sap/metrics-operator) and update accordingly.
repository: ghcr.io/sap/github.com/sap/metrics-operator/images/metrics-operator

charts/metrics-operator/values.yaml:8

  • The image repository path includes github.com in the URL, which looks unusual for a container registry. Please confirm the correct registry path (e.g., ghcr.io/sap/metrics-operator) and update accordingly.
repository: ghcr.io/sap/github.com/sap/metrics-operator/images/metrics-operator

README.md:157

  • [nitpick] Using "pod" as the metadata name may conflict with built-in Pod resources and reduce clarity in examples. Consider using a more descriptive and unique example name, such as metric-pod-count.
name: pod

@maximiliantech maximiliantech marked this pull request as ready for review June 3, 2025 14:35
@maximiliantech maximiliantech requested a review from mirzakopic June 3, 2025 14:50
@mirzakopic
Copy link
Contributor

make dev-local-all fails. can you please check why?

image

Copy link
Contributor

@mirzakopic mirzakopic left a comment

Choose a reason for hiding this comment

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

see comment above

@maximiliantech maximiliantech marked this pull request as draft June 5, 2025 15:14
@maximiliantech
Copy link
Member Author

Work in progress. Still cleaning up some stuff.

@maximiliantech maximiliantech marked this pull request as ready for review June 6, 2025 13:10
@maximiliantech
Copy link
Member Author

make dev-local-all fails. can you please check why?

@mirzakopic This error is resolved. I've also enhanced the README with more information on how to quickly get started. Looking forward to your review. Once this is merged, we can proceed in making the metrics-operator public from my side.

@maximiliantech
Copy link
Member Author

Additionally, I would suggest to make a release after this PR is merged. 🚀

maximilianbraun
maximilianbraun previously approved these changes Jun 6, 2025
Copy link
Member

@maximilianbraun maximilianbraun left a comment

Choose a reason for hiding this comment

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

Lgtm, an example with an open search and metrics ingestion would be great. Maybe as a follow up?

Copy link
Contributor

@moelsayed moelsayed left a comment

Choose a reason for hiding this comment

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

LGTM

@maximiliantech maximiliantech dismissed mirzakopic’s stale review June 10, 2025 13:50

Merging this PR now, because all concerns are addressed and reviewed by @moelsayed

@maximiliantech maximiliantech merged commit b6e4ba1 into main Jun 10, 2025
4 checks passed
@maximiliantech maximiliantech deleted the docs/improvements branch June 10, 2025 13:51
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.

4 participants