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

Fix Values comments #95

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Fix Values comments #95

merged 1 commit into from
Jul 25, 2024

Conversation

rdettai
Copy link
Contributor

@rdettai rdettai commented Jul 25, 2024

#92 introduced some inconistencies in the comments. This PRs fixes them.

@rdettai rdettai self-assigned this Jul 25, 2024
@rdettai rdettai requested a review from fmassot July 25, 2024 08:53
@rdettai
Copy link
Contributor Author

rdettai commented Jul 25, 2024

Also removed bootstrap.extraVolumes, bootstrap.extraVolumeMounts and bootstrap.initContainers which are dead code.

@rdettai
Copy link
Contributor Author

rdettai commented Jul 25, 2024

Side note: I find it very confusing that the settings for the bootstrap extraVolumes, extraVolumeMounts and initContainers got into a separate top level section called jobs. They belong to the bootstrap section. I don´t want to change it here because it's a breaking change, but I really think we should consider it.

@idrissneumann idrissneumann merged commit 01227e0 into main Jul 25, 2024
1 check passed
@idrissneumann idrissneumann deleted the fix-comments branch July 25, 2024 10:06
@idrissneumann
Copy link
Collaborator

Side note: I find it very confusing that the settings for the bootstrap extraVolumes, extraVolumeMounts and initContainers got into a separate top level section called jobs. They belong to the bootstrap section. I don't want to change it here because it's a breaking change, but I really think we should consider it.

Completely agree. This pr is merged but we can open a new issue and PR to do it later (it'll be a breaking change to document and version the right way though).

@idrissneumann
Copy link
Collaborator

@rdettai I opened an issuer for your suggestion, this way we'll see when it'll be prioritized etc: #98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants