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

added description for lifecycle parameter #482

Merged
merged 5 commits into from
Oct 23, 2023
Merged

added description for lifecycle parameter #482

merged 5 commits into from
Oct 23, 2023

Conversation

premkirank
Copy link
Contributor

@premkirank premkirank commented Oct 15, 2023

Description

Added a description for the lifecycle parameter in README

Issues Resolved

Fixes #459

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

@afrodidact afrodidact left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

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

@premkirank Thanks for your contribution. Changes looks good to me. Can you rebase with latest version of the chart. We released a new version yesterday so maybe you can build on top of that. Thanks

P:S There is a PR with similar changes raised 3 weeks ago so we need to wait on that PR. If that contributor can rebase we need to merge that change as that was raised first :)

#477

@premkirank premkirank requested a review from TheAlgo October 17, 2023 15:31
@prudhvigodithi
Copy link
Member

@premkirank can you please take a look at this comment. Thanks

@premkirank
Copy link
Contributor Author

@premkirank can you please take a look at this comment. Thanks

@prudhvigodithi I had rebased my PR and requested a review again. Is there anything else that I have missed?

@prudhvigodithi
Copy link
Member

Thanks, the CI tests fail can you please take care of it @premkirank ?

@premkirank
Copy link
Contributor Author

@prudhvigodithi I am not able to understand why the tests are failing. Please help here.

Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

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

@premkirank You have updated the appVersion to 2.12 and we don't have a version of OpenSearch yet. Can you revert this to 2.11. You only need to update chart version and not app version.

@premkirank
Copy link
Contributor Author

@premkirank You have updated the appVersion to 2.12 and we don't have a version of OpenSearch yet. Can you revert this to 2.11. You only need to update chart version and not app version.

@TheAlgo @prudhvigodithi have reverted the app version

@premkirank premkirank requested a review from TheAlgo October 22, 2023 09:56
Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

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

LGTM!

@TheAlgo
Copy link
Member

TheAlgo commented Oct 23, 2023

@prudhvigodithi Please do have a look when free. Thanks

@prudhvigodithi
Copy link
Member

Thanks @TheAlgo, @premkirank can you please backport this to 1.x branch as well? this is for the OpenSearch 1.x series.
Thank you

@prudhvigodithi prudhvigodithi added the backport 1.x Backport to 1.x branch after merging to main label Oct 23, 2023
@prudhvigodithi prudhvigodithi merged commit 0d334a0 into opensearch-project:main Oct 23, 2023
8 checks passed
@premkirank
Copy link
Contributor Author

Thanks @TheAlgo, @premkirank can you please backport this to 1.x branch as well? this is for the OpenSearch 1.x series. Thank you

@prudhvigodithi Here is the backport PR - #491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.x Backport to 1.x branch after merging to main first-time-contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing description in opensearch chart readme
5 participants