-
Notifications
You must be signed in to change notification settings - Fork 883
feat: Compute project build parameters #82
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
This modifies a prior migration which is typically forbidden, but because we're pre-production deployment I felt grouping would be helpful to future contributors. This adds database functions that are required for the provisioner daemon and job queue logic.
Adds a projectparameter package to compute build-time project values for a provided scope. This package will be used to return which variables are being used for a build, and can visually indicate the hierarchy to a user.
Codecov Report
@@ Coverage Diff @@
## main #82 +/- ##
==========================================
- Coverage 71.90% 71.76% -0.14%
==========================================
Files 88 89 +1
Lines 3417 3563 +146
Branches 55 55
==========================================
+ Hits 2457 2557 +100
- Misses 751 786 +35
- Partials 209 220 +11
Continue to review full report at Codecov.
|
} | ||
|
||
// Project parameters come third! | ||
projectParameters, err := db.GetParameterValuesByScope(ctx, database.GetParameterValuesByScopeParams{ |
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.
Following from the comment above - this projectParameters
is a confusing name, because we use: for _, projectParameter := range projectHistoryParameters {
above!
Some possible ideas:
- We could rename
projectParameters
here tooverriddenProjectParameters
- We could rename
projectParameter
in those loops toprojectHistoryParameter
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.
Yup, I renamed em' all to projectHistoryParameter
instead. Long name, but very clear!
// Validates and computes the value for parameters; setting the value on "parameterByName". | ||
func (c *compute) inject(scopedParameters []database.ParameterValue) error { |
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.
Good idea to factor this out to a helper method
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.
Just some minor nits about comments and naming for the projectHistoryParameters
/ projectParameters
when we compute the override - otherwise looks good to me!
Adds a projectparameter package to compute build-time project
values for a provided scope.
This package will be used to return which variables are being
used for a build, and can visually indicate the hierarchy to
a user.