-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat(application): Add support for Kustomize Patches #210
base: main
Are you sure you want to change the base?
feat(application): Add support for Kustomize Patches #210
Conversation
I'm a little unsure of the goverter conversions, there are a few fields missing that I have commented out. |
@stevendborrelli which fields are you uncertain about? https://github.com/argoproj/argo-cd/blob/d408909df6f950dd4b1a0b898c3b77259aef394f/pkg/apis/application/v1alpha1/types.go#L521 |
@blakepettersson There are new fields I was pulling from main that aren't in the last release. I've removed them in commit: 966a5ae. |
@janwillies @MisterMX could we get a review of this? |
Thanks for the PR @stevendborrelli! Changes look good to me, figuring out why the linter fails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me so far. I don't see any major issues - I haven't tested it by myself, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks already good to me. @stevendborrelli can you fix the one last thing and squash your changes into a single commit using the PR's title?
Signed-off-by: Steven Borrelli <[email protected]>
9d6941d
to
ad015b5
Compare
@MisterMX I've squashed the commits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you very much @stevendborrelli!
@stevendborrelli can you fix the linter issues? |
@MisterMX Yes, I will take a look. |
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Description of your changes
Fixes #208
I have:
RunIt looks like linting is broken in the Makefile.make reviewable test
to ensure this PR is ready for review.How has this code been tested
We've tested this successfully internally, also confirmed by comment #208 (comment).