Skip to content
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

stack: introduce hook.downloadEnabled boolean to include/exclude hook download job #86

Closed

Conversation

rpardini
Copy link
Contributor

stack: introduce hook.downloadEnabled boolean to include/exclude hook download job

  • sometimes we don't want to download the default hook binaries
  • this is separate from stack.hook.enabled, which controls the hostPath etc

… download job

- sometimes we don't want to download the default hook binaries
- this is separate from stack.hook.enabled, which controls the hostPath etc

Signed-off-by: Ricardo Pardini <[email protected]>
@rpardini rpardini force-pushed the stack-add-hook-downloadEnabled branch from 7634324 to fd93cb3 Compare March 23, 2024 19:20
Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rpardini , thanks for this! One small suggestion.

@@ -1,4 +1,5 @@
{{- if .Values.stack.hook.enabled }}
{{- if .Values.stack.hook.downloadEnabled }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could combine both these into a single statement.

Suggested change
{{- if .Values.stack.hook.downloadEnabled }}
{{- if and .Values.stack.hook.enabled .Values.stack.hook.downloadEnabled }}

@jacobweinstock jacobweinstock added the ready-to-merge Mergify: Ready for Merging label Jul 19, 2024
@jacobweinstock
Copy link
Member

Hey @rpardini, mergify is not able to merge this PR. Mind updating the PR to allow edits?

reference: https://github.com/tinkerbell/charts/pull/86/checks?check_run_id=27673082786

"This pull request cannot be embarked for merge
The merge queue pull request can't be updated
Details:

Pull request can't be updated with latest base branch changes

Mergify needs the author permission to update the base branch of the pull request.
@rpardini needs to authorize modification on its head branch."

@rpardini
Copy link
Contributor Author

I no longer have an use-case for this.

@rpardini rpardini closed this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants