-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Remove aliases for orchestrator #803
Remove aliases for orchestrator #803
Conversation
Prefer "strict" values for orchestrator, as it's easier to add aliases (if we think it's needed) than to remove them later. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Codecov Report
@@ Coverage Diff @@
## master #803 +/- ##
======================================
Coverage 50.9% 50.9%
======================================
Files 237 237
Lines 15338 15338
======================================
Hits 7808 7808
Misses 7028 7028
Partials 502 502 |
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.
LGTM
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.
LGTM 🐯
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.
LGTM, sorry @thaJeztah I interpreted it as "remove the case insensitive matcher", and not "remove all aliases" 😄
No worries; I thought that was the case; also was a bit in doubt between using |
Prefer "strict" values for orchestrator, as it's easier to add aliases (if we think it's needed) than to remove them later.
I commented on this in my review on #721 (comment), but looks like we forgot to actually make that change.
ping @vdemeester @silvin-lubecki PTAL
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)