Fix documentation on rollout specification for autoPromotionEnabled b…#4718
Fix documentation on rollout specification for autoPromotionEnabled b…#4718sncariad wants to merge 1 commit into
Conversation
…eing true As the autoPromotionEnabled is by default true when not specified the example in docs is misleading Signed-off-by: sncariad <148149940+sncariad@users.noreply.github.com>
|
Published E2E Test Results 4 files 4 suites 4h 2m 21s ⏱️ For more details on these failures, see this check. Results for commit 2db7c67. |
Published Unit Test Results2 469 tests 2 469 ✅ 3m 21s ⏱️ Results for commit 2db7c67. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4718 +/- ##
==========================================
- Coverage 85.03% 85.00% -0.04%
==========================================
Files 164 164
Lines 18989 18989
==========================================
- Hits 16148 16142 -6
- Misses 1993 1996 +3
- Partials 848 851 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kostis-codefresh
left a comment
There was a problem hiding this comment.
Actually it is the other way around. In most cases in the spec file the default is mentioned in the comments, and the example value shows how you OVERRIDE something.
Some quick examples:
minPodsPerReplicaSetdefault is 1, but the spec shows how to override to 2scaleDownDelayRevisionLimitdefault is nil, but the spec shows how to override to 2
So in the case of autoPromotionEnabled, the comment says default is true and the example value shows how to override with false.
It is true that there are some cases that don't follow this pattern (quick example would be progressDeadlineSeconds). But just changing autoPromotionEnabled on its own doesn't really clear up anything and it will be confusing to the next reader.
I believe a better PR would be to fix the remaining values in the spec to be uniform with everything else (i.e. mention default in comment and show example of override).



…eing true
As the autoPromotionEnabled is by default true when not specified the example in docs is misleading
Checklist:
"fix(controller): Updates such and such. Fixes #1234".