-
Notifications
You must be signed in to change notification settings - Fork 15.9k
Add registry.secretNames and registry.connections options to Helm chart #58094
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 registry.secretNames and registry.connections options to Helm chart #58094
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
f9165b4 to
d98ed0b
Compare
jscheffl
left a comment
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.
Thanks for the extension. I think this is OK and allows more options.
As you are deprecating an existing parameter, can you also please add a newsfragment in chart/newsfragments as RST describing the change such that it can be highlighted in release notes? Then people are (better) aware that they need to adjust parameters when upgrading.
d98ed0b to
e54ef7a
Compare
e54ef7a to
ca77b2f
Compare
|
Done! I added it under miscellaneous changes. Let me know if there’s anything else you’d like adjusted. Thanks!
|
jscheffl
left a comment
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.
Looks good to me, keeping this oopen for another 2 eyes of review to be merged in the next days
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
closes: #57615