Skip to content

Conversation

@soltysh
Copy link

@soltysh soltysh commented Oct 8, 2019

/assign @mfojtik
since you asked for it in openshift/cluster-kube-controller-manager-operator#285 (comment) and on top of that we use it in 3 operators: kcm-o, kas-o and oas-o.

/assign @deads2k

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 8, 2019

func MapToEnvVars(proxyConfig map[string]string) []corev1.EnvVar {
if proxyConfig == nil {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

i would return empty list maybe so we can append?

Copy link
Contributor

Choose a reason for hiding this comment

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

append works with nil too.

return []error(agg)
}

func MapToEnvVars(proxyConfig map[string]string) []corev1.EnvVar {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would make this env var name something generic

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I forgot about that 😉

return []error(agg)
}

func MapToEnvVars(proxyConfig map[string]string) []corev1.EnvVar {
Copy link
Contributor

Choose a reason for hiding this comment

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

godoc

@soltysh
Copy link
Author

soltysh commented Oct 14, 2019

@mfojtik updated, ptal

@mfojtik
Copy link
Contributor

mfojtik commented Oct 16, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2019
// MapToEnvVars converts a string-string map to a slice of corev1.EnvVar-s
func MapToEnvVars(mapEnvVars map[string]string) []corev1.EnvVar {
if mapEnvVars == nil {
return []corev1.EnvVar{}
Copy link
Contributor

Choose a reason for hiding this comment

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

why not nil?

Copy link
Author

Choose a reason for hiding this comment

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

@mfojtik convinced me it'll be easier to use this in append

Copy link
Contributor

Choose a reason for hiding this comment

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

it is not. This has smell. Don't do magic.

return []corev1.EnvVar{}
}

envVars := []corev1.EnvVar{}
Copy link
Contributor

Choose a reason for hiding this comment

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

make with capacity

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2019
@mfojtik
Copy link
Contributor

mfojtik commented Dec 5, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2019
@mfojtik mfojtik added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: mfojtik, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 73e1fb8 into openshift:master Dec 5, 2019
@soltysh soltysh deleted the maptoenv branch December 12, 2019 12:09
bertinatto pushed a commit to bertinatto/library-go that referenced this pull request Jul 2, 2020
Add MapToEnvVars helper function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants