-
Notifications
You must be signed in to change notification settings - Fork 212
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(config): syncing for postgres hook without secrets #2931
Conversation
Pull Request Test Coverage Report for Build 12113515963Details
💛 - Coveralls |
pkg/config/testdata/TestEmailDiff/local_disabled_remote_enabled.diff
Outdated
Show resolved
Hide resolved
Could you elaborate again why empty string was causing an issue? |
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.
Sorry for nitpicking. We want to be extremely strict about the coding standards for these configs. The impact of large frequent changes is very bad on cognitive load as well as maintainability, especially since we've spent a lot of time crafting it carefully.
pkg/config/testdata/TestEmailDiff/local_disabled_remote_enabled.diff
Outdated
Show resolved
Hide resolved
No worries thank's for the thorough review, I've changed the code accordingly to your comments. |
What kind of change does this PR introduce?
What is the current behavior?
Please link any relevant issues here.
What is the new behavior?
Feel free to include screenshots if it includes visual changes.
Additional context
Add any other context or screenshots.