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

code-refactoring: Gitops 3127 implement applicationset controller package #1011

Conversation

ciiay
Copy link
Collaborator

@ciiay ciiay commented Sep 29, 2023

What type of PR is this?
/kind code-refactoring

What does this PR do / why we need it:
Implemented notifications controller.

NOTE: this PR just adds the package files and tests, this code is not invoked at any point yet
Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:

Fixes #?
GITOPS-3127: https://issues.redhat.com/browse/GITOPS-3127

How to test changes / Special notes to the reviewer:
Unit tests and kuttl tests uploaded.

…-3127-implement-applicationset-controller-package
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
…-3127-implement-applicationset-controller-package
Signed-off-by: Yi Cai <[email protected]>
@ciiay ciiay requested a review from jaideepr97 September 29, 2023 19:12
common/cmds.go Outdated Show resolved Hide resolved
@jaideepr97
Copy link
Collaborator

@ciiay if all the old code has been accounted for, please remove those files
controllers/argocd/applicationset.go and controllers/argocd/applicationset_test.go

@ciiay ciiay requested a review from jaideepr97 October 2, 2023 21:10
@jaideepr97
Copy link
Collaborator

@ciiay Appset Also has a webhookserver route that needs to be reconciled by the appset controller
https://github.com/argoproj-labs/argocd-operator/blob/master/controllers/argocd/route.go#L294

@ciiay ciiay requested a review from jaideepr97 October 5, 2023 15:51
Copy link
Collaborator

@jaideepr97 jaideepr97 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 @ciiay !

@jaideepr97 jaideepr97 merged commit 7607d97 into argoproj-labs:operator-redesign Oct 5, 2023
1 of 3 checks passed
Julik24 pushed a commit to Julik24/argocd-operator that referenced this pull request Apr 24, 2024
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.

2 participants