Skip to content

Conversation

@gdasson
Copy link
Contributor

@gdasson gdasson commented Jan 20, 2024

Adding "reason" constants to the pkg repo as suggested in this PR: fluxcd/notification-controller#701

This will be followed by a PR in kustomize controller to refer to these constants from common pkg repo here and updates to the above mentioned PR to do the same.

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Left a few minor suggestions. Otherwise, the new constants look good to me.

@gdasson gdasson force-pushed the main branch 2 times, most recently from a44d758 to 81fee37 Compare January 30, 2024 05:44
@gdasson
Copy link
Contributor Author

gdasson commented Jan 30, 2024

@darkowlzz : Thanks for the review. I have incorporated your review comments: rebased, fixed and squashed my commits as well. Please review again and let me know if more updates are required. thx.

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks.

@gdasson
Copy link
Contributor Author

gdasson commented Feb 17, 2024

@stefanprodan , @hiddeco : Could you please review this PR? Its pending your review and approval. Thanks.

@stefanprodan
Copy link
Member

@gdasson please rebase with main and force push.

@gdasson
Copy link
Contributor Author

gdasson commented Mar 27, 2024

@stefanprodan: Done.

@stefanprodan stefanprodan added the area/api API related issues and pull requests label Mar 27, 2024
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @gdasson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api API related issues and pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants