Update KEP template with comment wrt to enablement/disablement tests in PRR section #3206
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
kind/kep
keps/NNNN-kep-template/README.md
Outdated
|
|
||
| Additionally, for features that are introducing a new API field, unit tests that | ||
| are exercising the `switch` of feature gate itself (what happens if I disable a | ||
| feature gate after having objects written with the new field) is also critical. |
There was a problem hiding this comment.
| feature gate after having objects written with the new field) is also critical. | |
| feature gate after having objects written with the new field) are also critical. |
| are exercising the `switch` of feature gate itself (what happens if I disable a | ||
| feature gate after having objects written with the new field) is also critical. | ||
| You can take a look at one potential example of such test in: | ||
| https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282 |
There was a problem hiding this comment.
Not sure if we need the example? Or maybe we should add an example to the first comment block too?
There was a problem hiding this comment.
Multiple people were telling me that they have no idea how to do that - after I was giving them a link it seems that it's actually happening. So I think it's useful.
Regarding the first block, we're much less prescriptive there - I'm fine with updating that, but maybe we can do that in a separate PR?
|
Adding a hold - I don't think the KEP template should be changing before the enhancements collection period ends. /hold |
k8s-ci-robot
added
the
do-not-merge/hold
kikisdeliveryservice
added
area/enhancements
wojtek-t
force-pushed
the
update_kep_template
branch
from
e10afc8 to
628974b
Compare
k8s-ci-robot
added
the
kind/kep
Sure - this can (and should) absolutely wait until then (given it's more-or-less almost now). |
|
looks good /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnbelamaric, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/lgtm I'll leave it to another reviewer to remove the hold |
k8s-ci-robot
added
the
lgtm
|
I think change will help people. /hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
/assign @deads2k @ehashman @johnbelamaric