-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(cli): add TURBO_CI_VENDOR_ENV_KEY #1622
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
feat(cli): add TURBO_CI_VENDOR_ENV_KEY #1622
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Set `TURBO_CI_VENDOR_ENV_KEY` to support vercel/turborepo#1622
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still learning to read Go syntax, so don't trust this review too much, but generally lgtm!
func getEnvPairsFromPrefixes(includePrefixes []string, excludePrefix string, allEnvVars map[string]string) []string { | ||
allEnvPairs := []string{} | ||
for _, includePrefix := range includePrefixes { | ||
hashableFrameworkEnvPairs := getEnvPairsFromPrefix(includePrefix, excludePrefix, allEnvVars) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably have inlined this logic, because I get confused when there are two functions named the same, but just a style preference (and I see that there were already two functions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely a path-dependence outcome. It'd have been fine the other way but the path we took to get here had this outcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good callout - I actually did this originally because I was going to call getEnvPairsFromPrefix in a goroutine, but decided it wasn't worth it since we're dealing with pretty small scale here.
for k, v := range allEnvVars { | ||
if strings.HasPrefix(k, prefix) { | ||
hashableFrameworkEnvPairs = append(hashableFrameworkEnvPairs, fmt.Sprintf("%v=%v", k, v)) | ||
if excludePrefix != "" && strings.HasPrefix(k, excludePrefix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Support blocklist for automatically include env vars