Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign updeprecate state field of eventSeries on Event API #75987
Conversation
k8s-ci-robot
added
release-note
kind/api-change
size/S
needs-sig
needs-priority
cncf-cla: yes
sig/apps
and removed
needs-sig
labels
Apr 1, 2019
k8s-ci-robot
requested review from
dchen1107 and
nikhiljindal
Apr 1, 2019
This comment has been minimized.
This comment has been minimized.
|
/assign @bgrant0607 @wojtek-t |
k8s-ci-robot
assigned
bgrant0607 and
wojtek-t
Apr 1, 2019
This comment has been minimized.
This comment has been minimized.
fejta-bot
commented
Apr 1, 2019
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
This comment has been minimized.
This comment has been minimized.
|
/label api-review |
k8s-ci-robot
added
priority/important-soon
sig/scalability
sig/instrumentation
api-review
and removed
needs-priority
labels
Apr 1, 2019
This comment has been minimized.
This comment has been minimized.
|
/retest |
This comment has been minimized.
This comment has been minimized.
|
Thanks for requesting an API review. Before proceeding, please ensure the prereq checklist has been completed, especially the items related to getting agreement on the change from the area owners, and having an associated KEP. This appears to be removing the field outright, rather than marking it as deprecated. Was this discussed and agreed to? |
This comment has been minimized.
This comment has been minimized.
We were discussing with @bgrant0607 and I think the decision was to deprecate. |
This comment has been minimized.
This comment has been minimized.
|
@wojtek-t @liggitt - I’m fine with this being subject to the depreciation policy, even though this was never really used somewhere. cc @bgrant0607 |
yastij
force-pushed the
yastij:event-state-deprecation
branch
from
1f939af
to
0dde81e
Apr 2, 2019
k8s-ci-robot
added
size/XS
and removed
size/S
labels
Apr 2, 2019
This comment has been minimized.
This comment has been minimized.
|
@liggitt - Planned for removal in 1.19 as per our deprecation policy and #65782 (comment) |
This comment has been minimized.
This comment has been minimized.
|
/cc @bgrant0607 @liggitt |
k8s-ci-robot
requested a review
from
bgrant0607
Apr 2, 2019
This comment has been minimized.
This comment has been minimized.
|
/retest |
yastij
force-pushed the
yastij:event-state-deprecation
branch
from
b63328e
to
87a0af9
Apr 16, 2019
k8s-ci-robot
added
size/S
and removed
size/XS
labels
Apr 16, 2019
This comment has been minimized.
This comment has been minimized.
|
@bgrant0607 @wojtek-t - our serialization test is failing as round tripping of v1 to our internal version, this due to the fact that the storage version is v1 (so we cannot drop it from storage version as we need it to round trip v1beta1). |
yastij
force-pushed the
yastij:event-state-deprecation
branch
from
87a0af9
to
8221fff
Apr 16, 2019
This comment has been minimized.
This comment has been minimized.
|
@yastij Yes, sorry, I didn't read the text I copied too carefully. I'm fine with deprecate now and remove in a couple releases. |
This comment has been minimized.
This comment has been minimized.
|
@bgrant0607 - less than year would make us deprecate this in 1.18. would you prefer to deprecate this in 1.19 to respect the deprecation policy ? |
liggitt
moved this from In progress
to Changes requested
in API Reviews
Apr 16, 2019
liggitt
moved this from Changes requested
to In progress
in API Reviews
Apr 16, 2019
liggitt
moved this from In progress
to Changes requested
in API Reviews
Apr 18, 2019
This comment has been minimized.
This comment has been minimized.
|
@yastij I'm fine with 1.18. |
This comment has been minimized.
This comment has been minimized.
|
/retest |
k8s-ci-robot
removed request for
dchen1107 and
nikhiljindal
Apr 28, 2019
yastij
force-pushed the
yastij:event-state-deprecation
branch
from
8221fff
to
0f3e9ca
Apr 28, 2019
k8s-ci-robot
added
size/XS
and removed
size/S
labels
Apr 28, 2019
This comment has been minimized.
This comment has been minimized.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
label
May 2, 2019
liggitt
moved this from Changes requested
to Completed, 1.15
in API Reviews
May 2, 2019
This comment has been minimized.
This comment has been minimized.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, yastij 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
label
May 2, 2019
This comment has been minimized.
This comment has been minimized.
|
/retest |
This comment has been minimized.
This comment has been minimized.
fejta-bot
commented
May 2, 2019
|
/retest Review the full test history for this PR. Silence the bot with an |
2 similar comments
This comment has been minimized.
This comment has been minimized.
fejta-bot
commented
May 2, 2019
|
/retest Review the full test history for this PR. Silence the bot with an |
This comment has been minimized.
This comment has been minimized.
fejta-bot
commented
May 3, 2019
|
/retest Review the full test history for this PR. Silence the bot with an |
yastij commentedApr 1, 2019
•
edited by liggitt
What type of PR is this?
/kind api-change
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: