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

refactor: cleanups for k8sutils workload package. create enums and move common functions #1449

Merged
merged 14 commits into from
Aug 18, 2024

Conversation

blumamir
Copy link
Collaborator

@blumamir blumamir commented Aug 17, 2024

This is another step in the way to move everything related to common things into the common packages, and avoid duplications and untyped usages in the project.

This PR:

  • adds more files to the workload package in k8sutils to differentiate between the different utils and enums
  • enums for the workload kinds
  • function to handle and convert workload kinds
  • rename function to common format
  • replace usages in controllers code to use enums where possible

There are countless places to go over and improve. This PR is touching on the most easy and straight forward cases I spotted.

return err
}

func WorkloadKindLowerCaseFromKind(pascalCase WorkloadKind) WorkloadKindLowerCase {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is equivalent to strings.ToLower. I'm not sure why we need types for both pascal and lowercase. We can have just the pascal one and use ToLower when needed in the utils functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, since it's used in several places, I choose to have a type for it as enum

odiglet/pkg/kube/instrumentation_ebpf/pods.go Show resolved Hide resolved
@blumamir blumamir merged commit a8a45e9 into odigos-io:main Aug 18, 2024
21 checks passed
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