Tim Hockin
unread,Nov 30, 2022, 6:54:10 PM11/30/22Sign in to reply to author
Sign in to forward
You do not have permission to delete messages in this group
Sign in to report message as abuse
Either email addresses are anonymous for this group or you need the view member email addresses permission to view the original message
to dev, kubernetes-sig-network
Hi everyone,
I’ve been thinking about and talking about this for a while, and I
wanted to write it down ASAP. It got a little long, sorry.
The current cycle of KEP freeze, code freeze, repeat is not
sustainable for me. For a multitude of reasons, it always comes down
to the last 2 weeks of each cycle being _ridiculously_ busy. I know I
am not alone in this, but if I am being honest, it is becoming
increasingly difficult for me to manage while also having other
responsibilities.
Here’s what I plan to do about it.
PR reviews (mostly code but also KEPs) usually take non-linear time
compared to their size. I usually make it a personal policy to review
PRs in order of their size - XS first, XXL last. This is partly to
encourage small PRs but also because I can usually squeeze a few small
and even medium sized PRs into gaps in my daily calendar. Large (and
XLarge, and XXLarge) sized PRs generally take _much_ longer (hours to
days) and I need to *make* time for those.
Because of this, the bigger PRs tend to get delayed reviews, which
means we push up against the deadlines, which creates stress (for
myself and PR authors), and sometimes (too often) we miss the deadline
and have to start over in a new cycle. This stinks for everyone
involved.
The obvious answer is smaller PRs. Sometimes this is possible, but
sometimes it’s not. I am a vocal proponent of NOT merging half of a
feature (e.g. API without implementation). For new APIs or
substantial changes, that automatically means large PRs. One approach
that can help is to aggressively use multiple commits to optimize for
review (it’s hard to say exactly what this means - it really depends
on the PR).
I am instituting some new policies _for myself_ with regards to PRs,
and I wanted to let people know ahead of the next cycle(s). Other
maintainers may choose to follow suit, and maybe we’ll make it a
project norm eventually, but for now, this is what I am imposing on
myself.
0) I will always try to service smaller PRs before larger ones.
The best way to get my review is to keep PRs small and well-organized.
PLEASE use commits to illustrate independent actions (e.g. “this
commit just renames things”, “this commit refactors”, “this commit
adds new functionality”) and, even better, try to break things like
refactoring and cleanups into distinct PRs. It’s WAY easier for me to
review 10 small PRs than 1 large one.
1) Just because I reviewed (or approved) your KEP doesn’t necessarily
mean I am signing up for the code review.
If you think your PR is going to be big and you want me to review it,
I need you to confirm with me that I am signed up for that. If I say
yes, I will do my very best to follow through on it. I will try to be
realistic and not say yes to too many things, but if you drop an
unplanned jumbo PR on me, I can’t promise that I will be able to
review it in time.
2) Big PRs (especially those that cross SIGs) need multiple reviewers.
Kubernetes is large and nobody is an expert in all areas (least of all
me). If you want me to review your jumbo PR, I need you to show me
who else is signed up to review (or at least who you are asking) and
what area(s) they are covering. Putting together a review team should
happen relatively early in the life cycle of large changes.
TBD: should the cross-SIG “review team” be part of the KEP becoming
“implementable”?
TBD: should “estimated size” be part of the KEP?
3) Big PRs should be reviewed incrementally.
We’ve experimented with “feature branches”, but I’m open to trying
simpler approaches. If you want me to review the code that will
ultimately add up to a jumbo PR, let’s agree on a process which allows
me to review the work _as it is happening_. For example, create a
fork repo and send PRs to it, so that I (and the review team) can
participate earlier than “it’s all done”.
For the things I sign up to review, I will offer regular and ad hoc
meeting “slots”, so we can jump on a video call and discuss
incremental changes and plans in real time, well before the deadline.
This requires that you, the author, do not procrastinate. If you save
it all up for the end, I can’t promise that I will be able to review
it in time.
Hopefully these changes will bring a little sanity to the chaos, and
result in less contributions missing the boat, at least not because of
me :)
Tim