-
Notifications
You must be signed in to change notification settings - Fork 43
Description
Context
Follow-up from PR #11980 which added environment variable mirroring from runner to agent container.
Minor Improvements
1. Pre-sort MirroredEnvVars at initialization time
In pkg/workflow/env_mirror.go:108-110, the GetMirroredEnvArgs() function sorts the variable list on every call:
sortedVars := make([]string, len(MirroredEnvVars))
copy(sortedVars, MirroredEnvVars)
sort.Strings(sortedVars)Suggestion: Either define MirroredEnvVars in sorted order in the source code, or sort once in an init() function. This is a minor optimization since the function is called once per workflow compilation.
2. Simplify test code variable extraction
In pkg/workflow/env_mirror_test.go:44-52, the variable name extraction could use strings.SplitN() for clarity:
// Current:
for j := 0; j < len(envAssignment); j++ {
if envAssignment[j] == '=' {
varSet[envAssignment[:j]] = true
break
}
}
// Simpler:
parts := strings.SplitN(envAssignment, "=", 2)
varSet[parts[0]] = true3. Consider adding PYENV_ROOT
Python environment setup sometimes uses PYENV_ROOT on certain runner configurations. Consider adding it to the mirrored variables list if relevant for Python workflows.
Priority
Low - these are minor code quality improvements, not functional issues.