-
Notifications
You must be signed in to change notification settings - Fork 0
Adds support for global env-vars #16
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
Conversation
Reviewer's Guide by SourceryThis pull request adds support for global environment variables in tasks. It modifies the task parsing logic to include global environment variables and updates the Runfile structure to accommodate these changes. Sequence diagram for task parsing with global environment variablessequenceDiagram
participant Ctx as Context
participant RF as Runfile
participant TP as TaskParser
participant PT as ParsedTask
Ctx->>TP: ParseTask(ctx, rf, task)
TP->>RF: Check Env
alt Env exists
TP->>TP: parseEnvVars(ctx, rf.Env)
TP->>TP: Merge globalEnv
end
TP->>TP: resolveDotEnvFiles()
TP->>TP: parseDotEnvFiles()
TP->>TP: parseEnvVars(ctx, task.Env)
TP->>PT: Return ParsedTask with merged Env
Class diagram for updated Runfile structureclassDiagram
class Runfile {
+string Version
+map Includes
+EnvVar Env
+string[] DotEnv
+map Tasks
}
class EnvVar
Runfile --> EnvVar : Env
Runfile --> string : DotEnv
Runfile --> map : Includes
Runfile --> map : Tasks
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @nxtcoder17 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| Version string `json:"version,omitempty"` | ||
| Includes map[string]IncludeSpec `json:"includes"` | ||
| Env EnvVar `json:"env,omitempty"` |
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.
suggestion: Add comment explaining global vs task-specific Env and DotEnv
Consider adding a comment to explain the difference between these global Env and DotEnv settings and their task-specific counterparts. This can help prevent confusion for future maintainers.
// Env defines global environment variables for all tasks.
// Task-specific Env settings override these global values.
Env EnvVar `json:"env,omitempty"`
Closes #15
Summary by Sourcery
Implement support for global environment variables in tasks, enabling tasks to utilize and override global environment settings. Add comprehensive tests to ensure the correct behavior of global environment variables in various scenarios.
New Features:
Tests: