Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactor] alert reconciler #102

Merged
merged 4 commits into from
May 6, 2024
Merged

Conversation

nicolastakashi
Copy link
Collaborator

@nicolastakashi nicolastakashi commented Apr 22, 2024

This pull request primarily updates the Go version from 1.20 to 1.22 and makes several changes to the codebase to improve its efficiency and readability. The major changes include updating the Go version in multiple GitHub workflow files and the Dockerfile, updating the Controller Tools version in the Makefile, refactoring the AlertSpec and Alert structs in alert_types.go, and making changes to the AlertReconciler struct in alert_controller.go.

Updates to Go version:

Updates to Makefile:

  • Makefile: Updated the Controller Tools version from v0.9.2 to v0.15.0.

Refactoring of AlertSpec and Alert structs:

  • apis/coralogix/v1alpha1/alert_types.go: Refactored the AlertSpec struct to simplify the ExtractCreateAlertRequest method and changed the receiver from in *AlertSpec to a *Alert. Added a NewAlert method to the Alert struct. [1] [2]

Changes to AlertReconciler struct:

Updates to dependencies:

  • go.mod: Updated the k8s.io/utils dependency version.

Signed-off-by: Nicolas Takashi <[email protected]>
Copy link
Contributor

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

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

Good initial work :)

I understand it won't happen in this PR, but having end2end tests would make the refactoring a lot easier :P

@nicolastakashi nicolastakashi force-pushed the chore/refactoring-alertreconciler branch 6 times, most recently from 633b664 to e0dd1eb Compare April 30, 2024 09:56
Copy link
Contributor

github-actions bot commented Apr 30, 2024

New Coverage 11.6% of statements
Patch Coverage 57.4% of changed statements (35/61)

Coverage provided by https://github.com/seriousben/go-patch-cover-action

Signed-off-by: Nicolas Takashi <[email protected]>
@nicolastakashi nicolastakashi force-pushed the chore/refactoring-alertreconciler branch from e0dd1eb to e518933 Compare April 30, 2024 09:59
@nicolastakashi nicolastakashi marked this pull request as ready for review April 30, 2024 10:26
@nicolastakashi nicolastakashi requested a review from a team as a code owner April 30, 2024 10:26
Copy link
Contributor

@povilasv povilasv left a comment

Choose a reason for hiding this comment

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

One minor nit, other than that LGTM

controllers/alphacontrollers/alert_controller.go Outdated Show resolved Hide resolved
OrNovo
OrNovo previously approved these changes May 5, 2024
@nicolastakashi nicolastakashi force-pushed the chore/refactoring-alertreconciler branch from 9a15a93 to 8edd393 Compare May 6, 2024 08:34
@nicolastakashi nicolastakashi requested a review from OrNovo May 6, 2024 09:14
matej-g
matej-g previously approved these changes May 6, 2024
Copy link
Contributor

@matej-g matej-g left a comment

Choose a reason for hiding this comment

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

Nice job 👊 , two minor things!

controllers/alphacontrollers/alert_controller.go Outdated Show resolved Hide resolved
@nicolastakashi nicolastakashi force-pushed the chore/refactoring-alertreconciler branch from 3f16272 to 89f207f Compare May 6, 2024 10:01
@nicolastakashi nicolastakashi requested a review from matej-g May 6, 2024 10:07
@nicolastakashi nicolastakashi merged commit 631c4a4 into main May 6, 2024
7 checks passed
@nicolastakashi nicolastakashi deleted the chore/refactoring-alertreconciler branch May 6, 2024 11:59
Copy link
Contributor

🎉 This PR is included in version 0.2.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants