-
Notifications
You must be signed in to change notification settings - Fork 0
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: initial scaffolding for directive validation and git-based directives #4
feat: initial scaffolding for directive validation and git-based directives #4
Conversation
b6be5b7
to
5573655
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## promotion-directives #4 +/- ##
=======================================================
Coverage ? 48.51%
=======================================================
Files ? 255
Lines ? 17897
Branches ? 0
=======================================================
Hits ? 8683
Misses ? 8778
Partials ? 436 ☔ View full report in Codecov by Sentry. |
5573655
to
534e188
Compare
.PHONY: codegen-docs | ||
codegen-docs: | ||
npm install -g @bitnami/readme-generator-for-helm | ||
bash hack/helm-docs/helm-docs.sh | ||
|
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.
No changes here. I just re-ordered the targets so they are defined in the same order they are executed when using make codegen
. I thought it made it slightly easier to follow.
sed -i.bak 's/\*bool/bool/g' ${out_file} | ||
sed -i.bak 's/\*string/string/g' ${out_file} |
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.
The tool uses pointers for anything optional, which is annoying AF for strings and bools (maybe ints, too). There wasn't a flag to change that behavior, so I resorted to this.
@@ -6,7 +6,7 @@ import ( | |||
"fmt" | |||
) | |||
|
|||
func init() { | |||
func init() { |
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.
Linter caught this.
@@ -92,4 +92,3 @@ func configToStruct[T any](c Config) (T, error) { | |||
|
|||
return result, nil | |||
} | |||
|
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.
Linter caught this.
@@ -17,7 +17,6 @@ func BuiltinsRegistry() DirectiveRegistry { | |||
// register and retrieve directives by name. | |||
type DirectiveRegistry map[string]Directive | |||
|
|||
|
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.
Linter caught this.
e3c3941
to
367ad99
Compare
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
534e188
to
7a314f8
Compare
…privileges Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
…f a dir Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@@ -47,11 +47,11 @@ func NewEngine( | |||
// Execute runs the provided list of directives in sequence. | |||
func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) { | |||
// TODO(hidde): allow the workDir to be restored from a previous execution. | |||
workDir, err := os.CreateTemp("", "run-") | |||
workDir, err := os.MkdirTemp("", "run-") |
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.
🤦 doh.
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.
We already talked through the details of this offline, so lets just get it merged. 💯
No description provided.