-
Notifications
You must be signed in to change notification settings - Fork 40
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?
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?
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]> update libraries for trivy scan issues Signed-off-by: Steven Borrelli <[email protected]> address linter issues Signed-off-by: Steven Borrelli <[email protected]> lint scan issues Signed-off-by: Steven Borrelli <[email protected]> linter fixes Signed-off-by: Steven Borrelli <[email protected]> update linter Signed-off-by: Steven Borrelli <[email protected]> trigger linter Signed-off-by: Steven Borrelli <[email protected]> test linter Signed-off-by: Steven Borrelli <[email protected]> update github action and go version Signed-off-by: Steven Borrelli <[email protected]>
1808442
to
3385ab8
Compare
I've addressed all linter issues I can find with 1.56.x, updated the GitHub action I'd like this PR to get merged as I think the Github runner is unable to run the Linter action.
If I run the linter locally it runs fine: $ docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:v1.56.2 golangci-lint run -v
level=info msg="[config_reader] Config search paths: [./ /app / /root]"
level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="[lintersdb] Active 43 linters: [asasalint asciicheck bidichk bodyclose contextcheck durationcheck errcheck errchkjson errorlint exhaustive exportloopref gocheckcompilerdirectives gochecksumtype goconst gocritic gocyclo gofmt goimports golint gosec gosimple gosmopolitan govet ineffassign loggercheck makezero misspell musttag nakedret nilerr noctx prealloc protogetter reassign rowserrcheck spancheck sqlclosecheck staticcheck testifylint unconvert unparam unused zerologlint]"
level=info msg="[lintersdb] Active presets: [bugs unused]"
level=info msg="[loader] Go packages loading at mode 575 (compiled_files|exports_file|imports|name|types_sizes|deps|files) took 32.356091014s"
level=warning msg="[runner] The linter 'golint' is deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner. Replaced by revive."
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 11.565583ms"
level=info msg="[linters_context/goanalysis] analyzers took 3m27.289949583s with top 10 stages: buildir: 1m14.041348757s, goimports: 1m11.358363532s, buildssa: 18.754367116s, gocritic: 7.997533501s, fact_deprecated: 5.444047237s, inspect: 5.053023385s, exhaustive: 4.455594536s, ctrlflow: 3.67664383s, printf: 3.661249716s, nilness: 2.735253722s"
level=info msg="[runner] Issues before processing: 1019, after processing: 0"
level=info msg="[runner] Processors filtering stat (out/in): path_prettifier: 1019/1019, skip_dirs: 39/39, autogenerated_exclude: 39/39, exclude: 39/39, identifier_marker: 39/39, nolint: 0/16, cgo: 1019/1019, filename_unadjuster: 1019/1019, skip_files: 39/1019, exclude-rules: 16/39"
level=info msg="[runner] processing took 7.229796ms with stages: nolint: 3.266334ms, autogenerated_exclude: 1.389042ms, identifier_marker: 804µs, filename_unadjuster: 589.667µs, cgo: 433.459µs, skip_files: 244.251µs, path_prettifier: 242.583µs, exclude-rules: 209.167µs, skip_dirs: 47.833µs, max_same_issues: 834ns, severity-rules: 458ns, diff: 417ns, uniq_by_line: 375ns, fixer: 375ns, source_code: 208ns, path_prefixer: 167ns, sort_results: 167ns, exclude: 167ns, path_shortener: 125ns, max_from_linter: 125ns, max_per_file_from_linter: 42ns"
level=info msg="[runner] linters took 38.472752891s with stages: goanalysis_metalinter: 38.465446225s"
level=info msg="File cache stats: 78 entries of total size 831.5KiB"
level=info msg="Memory: 698 samples, avg is 1316.4MB, max is 2889.2MB"
level=info msg="Execution took 1m10.845190697s"
$ echo $?
0 |
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).