Skip to content

Conversation

Daniel-GrunbergerCA
Copy link
Collaborator

No description provided.

Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
@codiumai-pr-agent-free
Copy link

PR Analysis

  • 🎯 Main theme: Fixing the updateResourceKind function to handle the "ingress" case correctly
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: True
  • Focused PR: Yes, the PR is focused on fixing the updateResourceKind function and adding tests for it.

PR Feedback

  • General suggestions: The PR seems to be well structured and focused. The addition of tests for various cases is commendable. However, it would be beneficial to include a description in the PR to provide context and explain the changes made.

  • 🤖 Code feedback:

    • relevant file: k8sinterface/k8sdiscovery.go
      suggestion: Consider using a switch statement instead of multiple if statements for better readability and performance [medium]
      relevant line:

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@Daniel-GrunbergerCA Daniel-GrunbergerCA merged commit 8f50341 into main Aug 14, 2023
@Daniel-GrunbergerCA Daniel-GrunbergerCA deleted the fix-update-func branch August 14, 2023 12:51
@github-actions
Copy link

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

Comment on lines +346 to +348
if resource == "ingress" {
return "ingresses"
}
Copy link
Contributor

@YiscahLevySilas1 YiscahLevySilas1 Aug 14, 2023

Choose a reason for hiding this comment

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

@Daniel-GrunbergerCA maybe we should change this to be more generic, like:
if !strings.HasSuffix(resource, "ss"){ return fmt.Sprintf("%ses", resource) // add 'es' at the end of a resource }
there are some other k8s resources that end in ss (StorageClass for .g) . we don't check any of them in any controls but it makes sense to be prepared..

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