Hi,
Thank you David for bringing this up!
I am in favour of automatically closing stale PRs. I agree with Guozhang
that notifications of staleness to authors would be better than silently
closing them. I assume the notification happens automatically when the
label "Stale" is added to the PR.
+1 for Matthias' proposal of non-committers doing a pre-review. That
would definitely save some time for committer reviews.
Nikolay, great that you are willing to do reviews. We do not have a
separate list of PRs that need pre-reviews. You can consult the list of
PRs of Apache Kafka (https://github.com/apache/kafka/pulls) and choose
from there. I think that is the simplest way to start reviewing. Maybe
Luke has some tips here since he does an excellent job in reviewing as a
non-committer.
Best,
Bruno
On 07.02.22 08:24, Nikolay Izhikov wrote:
Hello, Matthias, Luke.
I agree with Matthias that contributors could and should help do more
"pre-review" PRs.
I, personally, ready to do the initial review of PRs. Do we have some recipe to
filter PRs that has potential to land in trunk?
Can, you, please, send me list of PRs that need to be pre-reviewed?
I might be useful thought to just do a better job to update KIP status more
frequently
First, I thought it’s an author job to keep KIP status up to date.
But, it can be tricky to determine actual KIP status because of lack of
feedback from committers :)
Second - the other issue is determine - what KIP just wait for a hero to
implement it, and what just wrong idea or something similar.
All of this kind of KIPs has status «Under discussion».
Actually, if someone has a list of potentially useful KIPS - please, send it.
I’m ready to work on one of those.
7 февр. 2022 г., в 05:28, Luke Chen <show...@gmail.com> написал(а):
I agree with Matthias that contributors could and should help do more
"pre-review" PRs.
Otherwise, we're not fixing the root cause of the issue, and still keeping
piling up the PRs (and auto closing them after stale)
And I also agree with Guozhang that we should try to notify at least the
committers about the closed PRs (maybe PR participants + committers if
possible).
Although the PRs are stale, there might be some good PRs just got ignored.
Thank you.
Luke
On Mon, Feb 7, 2022 at 6:50 AM Guozhang Wang <wangg...@gmail.com> wrote:
Thanks for bringing this up David. I'm in favor of some automatic ways to
clean up stale PRs. More specifically:
* I think there are indeed many root causes why we have so many stale PRs
that we should consider, and admittedly the reviewing manpower cannot keep
up with the contributing pace is a big one of them. But in this discussion
I'd personally like to keep this out of the scope and maybe keep it as a
separate discussion (I think we are having some discussions on some of
these root causes in parallel at the moment).
* As for just how to handle the existing stale PRs, I think having an
automatic way would be possibly the most effective manner, as I suspect how
maintainable it would be to do that manually. The question though would be:
do we just automatically close those PRs silently or should we also send
notifications along with it. It seems https://github.com/actions/stale can
definitely do the first, but not sure if it could the second? Plus let's
say if we want notifications and it's doable via Action, could we configure
just the committers list (as sending notifications to all community
subscribers may be too spammy)? Personally I feel setting 6 months for
closing and notifying committers on a per-week basis seems sufficient.
Guozhang
On Sun, Feb 6, 2022 at 9:58 AM Matthias J. Sax <mj...@apache.org> wrote:
I am +1 to close stale PRs -- not sure to what extend we want to
automate it, or just leave it up to the committers to do the cleanup
manually. I am happy both ways.
However, I also want to point out, that one reason why we have so many
stale PRs is the committer overload to actually review PRs. It's a pity
that committer cannot keep up with the load (guilty as charged myself).
Not sure if it would help if more contributors could help doing reviews,
such that PRs are "pre-reviewed" and already in good shape before a
committer reviews it?
For KIPs, there is actually two more categories:
- "Dormant/Inactive"
- "Discarded:
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-DiscardedKIPs
For Kafka Streams in particular, we also try to make the status of KIP
clear in the corresponding sub-page:
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams
I might be useful thought to just do a better job to update KIP status
more frequently -- we could also re-organize the main KIP wiki page -- I
think it contains too much information and is hard to read.
For the Kafka Streams sub-page, we use it for all "active" KIPs, while
we maintain a second page for adopted KIPs grouped by release:
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams+KIP+Overview
I find this much more digestible compared to the main KIP page.
Might also be good to have a sub-page for Connect KIPs?
-Matthias
On 2/5/22 05:57, Luke Chen wrote:
Hi Nikolay,
That's a good question!
But I think for stale KIP, we should have another discussion thread.
In my opinion, I agree we should also have similar mechanism for KIP.
Currently, the state of KIP has "under discussion", "voting", and
"accepted".
The KIP might stay in "discussion" or "voting" state forever.
We might be able to have a new state called "close" for KIP.
And we can review those inactive KIPs for a long time like PR did, to
see
if these KIPs need to close or re-start the discussion again.
Thank you.
Luke
On Sat, Feb 5, 2022 at 9:23 PM Nikolay Izhikov <nizhi...@apache.org>
wrote:
Hello, David, Luke.
What about KIPs?
Should we have some special state on KIPs that was rejected or can’t
be
implemented due to lack of design or when Kafka goes in another
direction?
Right now those kind of KIPs just have no feedback.
For me as a contributor it’s not clear - what is wrong with the KIP.
Is it wrong? Is there are no contributor to do the implementation?
5 февр. 2022 г., в 15:49, Luke Chen <show...@gmail.com> написал(а):
Hi David,
I agree with it! This is also a good way to let both parties (code
author
and reviewers) know there's a PR is not active anymore. Should we
continue
it or close it directly?
In my opinion, 1 year is too long, half a year should be long enough.
Thank you.
Luke
On Sat, Feb 5, 2022 at 8:17 PM Sagar <sagarmeansoc...@gmail.com>
wrote:
Hey David,
That's a great idea.. Just to stress your point, this keeps both
parties
informed if a PR has become stale. So, the reviewer would know that
there
was some PR which was being reviewed but due to inactivity it got
closed so
maybe time to relook and similarly the submitter.
And yeah, any stale/unused PRs can be closed straight away thereby
reducing
the load on reviewers. I have done some work on kubernetes open
source
and
they follow a similar paradigm which is useful.
Thanks!
Sagar.
--
-- Guozhang
