Skip to content

chore: upgrade go version, add tests and comments, and fix arg names #396

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

Merged
merged 8 commits into from
Apr 14, 2025

Conversation

MeNsaaH
Copy link
Collaborator

@MeNsaaH MeNsaaH commented Apr 12, 2025

  • add extra tests
  • add some comments to function
  • fix args spelling error max-concurrenct-checks -> max-concurrent-checks
  • Add namespace to generated-store -> kubepug-generated-store
  • Fix golang-ci lint issues by running golangci-lint run --fix and also manually fixing other cases.

Signed-off-by: Mmadu Manasseh <[email protected]>
@MeNsaaH MeNsaaH requested a review from Copilot April 12, 2025 23:46
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

pkg/kustomize/process.go:50

  • Switching the logging level from Info to Debug may reduce the visibility of directory processing logs in production. Please confirm that this change is intentional for operational monitoring.
log.Debug().Msgf("processing directory %q", relBase)

@MeNsaaH MeNsaaH requested a review from djeebus April 12, 2025 23:48
Signed-off-by: Mmadu Manasseh <[email protected]>
Copy link

github-actions bot commented Apr 12, 2025

Temporary image deleted.

MeNsaaH added 4 commits April 13, 2025 00:55
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
Signed-off-by: Mmadu Manasseh <[email protected]>
@MeNsaaH MeNsaaH requested a review from Copilot April 13, 2025 00:44
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 17 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • .tool-versions: Language not supported
  • go.mod: Language not supported
  • tools/dump_crds/go.mod: Language not supported
Comments suppressed due to low confidence (2)

pkg/argo_client/kustomize.go:38

  • The use of log.Printf for error logging is inconsistent with the rest of the codebase that uses zerolog. Consider using log.Error().Err(err).Msg(...) for consistency.
defer func() { if err := r.Close(); err != nil { log.Printf("failed to close file: %v", err) } }() // ignore error: file was opened read-only.

pkg/config/config.go:88

  • Despite updating the mapstructure tag, the internal field name 'MaxConcurrenctChecks' remains misspelled. Consider renaming it to 'MaxConcurrentChecks' for clarity and consistency.
MaxConcurrenctChecks     int           `mapstructure:"max-concurrent-checks"`

Signed-off-by: Mmadu Manasseh <[email protected]>
@MeNsaaH MeNsaaH requested a review from Copilot April 13, 2025 00:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 19 out of 21 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • .tool-versions: Language not supported
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

pkg/checks/preupgrade/kubepug.go:54

  • The format specifier '%b' converts the index 'i' into its binary representation; if a decimal representation is intended, consider using '%d' instead.
tmpFile := fmt.Sprintf("%s/%b.yaml", tempDir, i)

@MeNsaaH MeNsaaH changed the title chore: Refactor to add tests, comments and fix some arg names chore: upgrade go version, add tests and comments, and fix arg names Apr 13, 2025
@MeNsaaH MeNsaaH requested a review from Greyeye April 14, 2025 16:24
@MeNsaaH MeNsaaH merged commit 921e351 into main Apr 14, 2025
5 checks passed
@MeNsaaH MeNsaaH deleted the chore/refactor branch April 14, 2025 21:30
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