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

Add initContainer in statefulsets and values #196

Merged
merged 6 commits into from
Oct 31, 2023
Merged

Conversation

ProjectsOfMLee
Copy link
Contributor

@ProjectsOfMLee ProjectsOfMLee commented Oct 25, 2023

@JorgeGar Do I need to also add the specific initContainer configs solution to #194 to this PR or leave it customizable as is?

@ProjectsOfMLee ProjectsOfMLee marked this pull request as draft October 25, 2023 18:29
@ProjectsOfMLee ProjectsOfMLee marked this pull request as ready for review October 25, 2023 18:47
@JorgeGar
Copy link
Contributor

@JorgeGar Do I need to also add the specific initContainer configs solution to #194 to this PR or leave it customizable as is?

Hi @ProjectsOfMLee, you need to make so the default value generates the same result that you had before your change. I see in your commits this is what you did, so that’s perfect.

A part from that, you do need to add the initContainert to the HelmSetup.md.

@abeizn
Copy link
Contributor

abeizn commented Oct 26, 2023

@ZhangNing10

@ProjectsOfMLee
Copy link
Contributor Author

@JorgeGar Do I need to also add the specific initContainer configs solution to #194 to this PR or leave it customizable as is?

Hi @ProjectsOfMLee, you need to make so the default value generates the same result that you had before your change. I see in your commits this is what you did, so that’s perfect.

A part from that, you do need to add the initContainert to the HelmSetup.md.

Thanks! I have added it to the .md

@JorgeGar
Copy link
Contributor

@JorgeGar Do I need to also add the specific initContainer configs solution to #194 to this PR or leave it customizable as is?

Hi @ProjectsOfMLee, you need to make so the default value generates the same result that you had before your change. I see in your commits this is what you did, so that’s perfect.
A part from that, you do need to add the initContainert to the HelmSetup.md.

Thanks! I have added it to the .md

Great! I’m checking with my phone, but it seems like the tabulation/number-of-spaces is different from the rest. Could you kindly double-check and make sure it’s the same? It’s a silly thing, but it makes future reviews easier. The rest of the PR looks good to me 😄

Copy link
Contributor

@JorgeGar JorgeGar left a comment

Choose a reason for hiding this comment

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

LGTM

@ProjectsOfMLee
Copy link
Contributor Author

@JorgeGar Do I need to also add the specific initContainer configs solution to #194 to this PR or leave it customizable as is?

Hi @ProjectsOfMLee, you need to make so the default value generates the same result that you had before your change. I see in your commits this is what you did, so that’s perfect.
A part from that, you do need to add the initContainert to the HelmSetup.md.

Thanks! I have added it to the .md

Great! I’m checking with my phone, but it seems like the tabulation/number-of-spaces is different from the rest. Could you kindly double-check and make sure it’s the same? It’s a silly thing, but it makes future reviews easier. The rest of the PR looks good to me 😄

Screenshot 2023-10-26 at 11 40 48 AM

Have adjusted the tabs. Thank you :)

@cicdguy
Copy link

cicdguy commented Oct 30, 2023

Can we get this into a beta release, please? We are eagerly anticipating this to be available soon! Thanks @JorgeGar, @ProjectsOfMLee and team!

@ZhangNing10
Copy link
Contributor

LGTM

@abeizn abeizn merged commit c5fcec6 into apache:main Oct 31, 2023
3 checks passed
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.

5 participants