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(cmd/root): simplify parseEnvs #2162

Merged
merged 2 commits into from
Jan 24, 2024
Merged

refactor(cmd/root): simplify parseEnvs #2162

merged 2 commits into from
Jan 24, 2024

Conversation

Juneezee
Copy link
Contributor

Currently parseEnvs accept two parameters:

  1. env []string
  2. envs map[string]string

parseEnvs then do a safety nil check for env.

act/cmd/root.go

Lines 299 to 312 in 15bb54f

func parseEnvs(env []string, envs map[string]string) bool {
if env != nil {
for _, envVar := range env {
e := strings.SplitN(envVar, `=`, 2)
if len(e) == 2 {
envs[e[0]] = e[1]
} else {
envs[e[0]] = ""
}
}
return true
}
return false
}

However, we never pass a nil env to parseEnvs in newRunCommand.

act/cmd/root.go

Lines 417 to 418 in 15bb54f

envs := make(map[string]string)
_ = parseEnvs(input.envs, envs)

act/cmd/root.go

Lines 422 to 423 in 15bb54f

inputs := make(map[string]string)
_ = parseEnvs(input.inputs, inputs)

This commit simplify the parseEnvs function to accept just one env []string parameter and return the result as map[string]string instead.

@Juneezee Juneezee requested a review from a team as a code owner January 20, 2024 09:32
Copy link
Contributor

mergify bot commented Jan 20, 2024

@Juneezee this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Jan 20, 2024
Copy link
Contributor

mergify bot commented Jan 20, 2024

@Juneezee this pull request has failed checks 🛠

Prior to this commit, `parseEnvs` accept two parameters:

  1. env []string
  2. envs map[string]string

`parseEnvs` then do a `nil` check for `env`. However, we never pass a
`nil` `env` to `parseEnvs` in `newRunCommand`.

This commit simplify the `parseEnvs` function to accept just one
`env []string` parameter and return the result as `map[string]string`
instead.

Signed-off-by: Eng Zer Jun <[email protected]>
Copy link

codecov bot commented Jan 21, 2024

Codecov Report

Attention: 1028 lines in your changes are missing coverage. Please review.

Comparison is base (4989f44) 61.22% compared to head (75260b8) 60.90%.
Report is 306 commits behind head on master.

Files Patch % Lines
pkg/artifactcache/handler.go 65.32% 103 Missing and 43 partials ⚠️
pkg/runner/run_context.go 73.37% 75 Missing and 19 partials ⚠️
pkg/runner/expression.go 55.17% 66 Missing and 12 partials ⚠️
pkg/container/docker_run.go 1.47% 66 Missing and 1 partial ⚠️
pkg/runner/action_cache.go 50.74% 49 Missing and 17 partials ⚠️
pkg/container/docker_network.go 0.00% 56 Missing ⚠️
pkg/model/planner.go 28.57% 53 Missing and 2 partials ⚠️
pkg/model/workflow.go 52.72% 42 Missing and 10 partials ⚠️
pkg/runner/reusable_workflow.go 52.47% 42 Missing and 6 partials ⚠️
pkg/common/outbound_ip.go 0.00% 44 Missing ⚠️
... and 27 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2162      +/-   ##
==========================================
- Coverage   61.22%   60.90%   -0.33%     
==========================================
  Files          46       53       +7     
  Lines        7141     8990    +1849     
==========================================
+ Hits         4372     5475    +1103     
- Misses       2462     3074     +612     
- Partials      307      441     +134     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mergify mergify bot removed the needs-work Extra attention is needed label Jan 21, 2024
@mergify mergify bot merged commit 424fd5e into nektos:master Jan 24, 2024
10 checks passed
jmikedupont2 pushed a commit to meta-introspector/act that referenced this pull request Mar 10, 2024
Prior to this commit, `parseEnvs` accept two parameters:

  1. env []string
  2. envs map[string]string

`parseEnvs` then do a `nil` check for `env`. However, we never pass a
`nil` `env` to `parseEnvs` in `newRunCommand`.

This commit simplify the `parseEnvs` function to accept just one
`env []string` parameter and return the result as `map[string]string`
instead.

Signed-off-by: Eng Zer Jun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants