Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed most of core-related code owners #5917

Merged
merged 1 commit into from
Aug 9, 2022
Merged

Removed most of core-related code owners #5917

merged 1 commit into from
Aug 9, 2022

Conversation

mmedenjak
Copy link
Contributor

@mmedenjak mmedenjak commented Aug 9, 2022

Cover letter

Since introducing required reviewers for core code, we have found that
it has led to more problems than benefit:

  • most of the time too many people get pinged to review a PR, and not
    everyone needs to actually review it (example - Partition balancer backend #5371)
  • even when the PR author reqests someone for review, the requested
    reviewer sometimes misses the fact that they were asked specifically and
    not due to an over-zealous CODEOWNERS file. As such, even the list of
    PRs where one is requested on (https://github.com/pulls/review-requested)
    has too much noise to be trustworthy
  • sometimes people don't do a review since they see others were
    requested automatically, which doesn't help with spreading knowledge
  • usually the PR author can quickly determine who to request so that's
    good enough going forward

Backport Required

  • papercut/not impactful enough to backport

UX changes

N/A

Release notes

  • none

Features

N/A

Improvements

N/A

Since introducing required reviewers for core code, we have found that
it has led to more problems than benefit:
-  most of the time too many people get pinged to review a PR, and not
everyone needs to actually review it
- even when the PR author reqests someone for review, the requested
reviewer sometimes misses the fact that they were asked specifically and
not due to an over-zealous CODEOWNERS file. As such, even the list of
PRs where one is requested on (https://github.com/pulls/review-requested)
has too much noise to be trustworthy
- sometimes people don't do a review since they see others were
requested automatically, which doesn't help with spreading knowledge
- usually the PR author can quickly determine who to request so that's
good enough going forward
@mmedenjak mmedenjak requested a review from a team as a code owner August 9, 2022 11:24
@mmedenjak mmedenjak requested review from gousteris and removed request for a team August 9, 2022 11:24
@CLAassistant
Copy link

CLAassistant commented Aug 9, 2022

CLA assistant check
All committers have signed the CLA.

#
/.vim @dotnwat
/.dir-locals.el @graphcareful
licenses @emaxerrno @dswang
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes:

If we want to keep some of these, here is a proxy metric to who should be owning these (number of commits):

$ git shortlog -s -n --all CONTRIBUTING.md 
     3  Alexander Gallego
     2  David Castillo
     2  Rob Blafford
$ git shortlog -s -n --all .vim/
     2  Noah Watkins
$ git shortlog -s -n --all .dir-locals.el
     7  Alexander Gallego
     2  Rob Blafford
     1  Andrew Hsu
     1  Kostas Kyrimis

#
# core
#
/src/js @andresaristizabal
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all of core because of the reasons in the cover letter and because I believe every PR author can easily find someone to review, at least by using git blame. If we want to keep some of these, here is a proxy metric to who should be owning these (number of commits):

$ git shortlog -s -n --all src/js/
   131  Andres Aristizabal
    47  Kostas Kyrimis
    25  Rob Blafford

$ git shortlog -s -n --all src/v/archival
    81  Evgeny Lazin
    51  Alexey Zatelepin
    20  John Spray

$ git shortlog -s -n --all src/v/bytes/
    80  Alexander Gallego
    52  Noah Watkins
     8  Duarte Nunes

$ git shortlog -s -n --all src/v/cloud_storage/
   111  Evgeny Lazin
    56  Elena Anyusheva
    36  Alexey Zatelepin

$ git shortlog -s -n --all src/v/cluster/
   747  Michal Maslanka
   450  Noah Watkins
   340  Denis Rystsov
   196  John Spray
   117  Alexander Gallego

$ git shortlog -s -n --all src/v/compression/
    28  Alexander Gallego
    19  Noah Watkins

$ git shortlog -s -n --all src/v/config/
   128  John Spray
    65  Noah Watkins
    62  Michal Maslanka
    34  Alexander Gallego
    30  Denis Rystsov

$ git shortlog -s -n --all src/v/coproc/
   274  Rob Blafford
    39  Noah Watkins
    18  Robert Blafford
    17  Ben Pope

$ git shortlog -s -n --all src/v/finjector/
    16  Alexander Gallego
     2  Michal Maslanka
     2  Noah Watkins

$ git shortlog -s -n --all src/v/hashing/
    26  Alexander Gallego
    12  Noah Watkins
     7  Duarte Nunes

$ git shortlog -s -n --all src/v/http/
    23  Evgeny Lazin
    12  Noah Watkins
     5  Abhijat Malviya
     4  Ben Pope

$ git shortlog -s -n --all src/v/json
    19  Ben Pope
     5  Noah Watkins
     4  Elena Anyusheva
     3  Andrew Hsu

$ git shortlog -s -n --all src/v/kafka/
   671  Noah Watkins
   337  Michal Maslanka
   168  Ben Pope
   117  Denis Rystsov
    90  Rob Blafford
    82  Alexander Gallego

$ git shortlog -s -n --all src/v/kafka/client/
   103  Ben Pope
    45  Noah Watkins
    16  Rob Blafford
     9  John Spray
     7  Michal Maslanka

$ git shortlog -s -n --all src/v/model/
   112  Alexander Gallego
    80  Noah Watkins
    68  Michal Maslanka
    24  Denis Rystsov
    23  Duarte Nunes
    14  Evgeny Lazin

$ git shortlog -s -n --all src/v/pandaproxy/
   578  Ben Pope
    34  Noah Watkins
    26  John Spray
     8  Michal Maslanka
     6  Alexander Gallego

$ git shortlog -s -n --all src/v/platform/
     3  Alexander Gallego
     2  Vadim Plakhtinskiy
     1  Andrew Hsu
     1  John Spray

$ git shortlog -s -n --all src/v/prometheus/
     5  Alexander Gallego
     3  Duarte Nunes
     3  Kostas Kyrimis

$ git shortlog -s -n --all src/v/raft/
   693  Michal Maslanka
   228  Alexander Gallego
   180  Noah Watkins
    77  Denis Rystsov
    75  John Spray
    44  Michał Maślanka

$ git shortlog -s -n --all src/v/random/
    16  Alexander Gallego
     7  Noah Watkins
     5  Duarte Nunes
     3  Michal Maslanka
     3  Ben Pope
     2  Rob Blafford

$ git shortlog -s -n --all src/v/redpanda/
   240  Noah Watkins
   170  Michal Maslanka
   140  John Spray
    99  Alexander Gallego
    67  Ben Pope
    64  Duarte Nunes
    35  Rob Blafford
    28  Vadim Plakhtinskiy
    21  Denis Rystsov
    20  Evgeny Lazin
    14  Michał Maślanka
    14  Alexey Zatelepin
    13  Andrew Wong
     9  Vlad Lazar

$ git shortlog -s -n --all src/v/reflection/
     5  Alexander Gallego
     5  Rob Blafford
     4  Kostas Kyrimis
     4  Denis Rystsov
     4  Noah Watkins

$ git shortlog -s -n --all src/v/resource_mgmt/
    12  Alexander Gallego
    12  Michal Maslanka
     6  Noah Watkins
     3  Evgeny Lazin
     2  Rob Blafford

$ git shortlog -s -n --all src/v/rpc/
   169  Alexander Gallego
   132  Michal Maslanka
    90  Noah Watkins
    25  John Spray
    12  Ben Pope
    10  Evgeny Lazin

$ git shortlog -s -n --all src/v/s3
    32  Evgeny Lazin
    11  Ben Pope
     8  Abhijat Malviya
     6  Noah Watkins
     3  Michal Maslanka

$ git shortlog -s -n --all src/v/security/
    30  Noah Watkins
    12  Ben Pope
     9  Michal Maslanka
     7  Rob Blafford
     4  John Spray
     4  Andrew Wong

$ git shortlog -s -n --all src/v/serde/
    24  Felix Gündling
    16  Noah Watkins
     9  Kefu Chai
     4  Bharath Vissapragada

$ git shortlog -s -n --all src/v/ssx/
     8  Rob Blafford
     5  Noah Watkins
     5  Ben Pope
     4  Vlad Lazar
     4  Kefu Chai
     3  John Spray

$ git shortlog -s -n --all src/v/storage/
   483  Alexander Gallego
   278  Noah Watkins
   261  Michal Maslanka
    81  John Spray
    43  Duarte Nunes
    39  Alexey Zatelepin
    31  Ben Pope
    22  Kostas Kyrimis

$ git shortlog -s -n --all src/v/syschecks/
    17  Alexander Gallego
     4  Duarte Nunes
     3  Noah Watkins
     3  Ben Pope
     1  Ivo Jimenez
     1  Kefu Chai

$ git shortlog -s -n --all src/v/test_utils/
    18  Alexander Gallego
    14  Noah Watkins
    11  Michal Maslanka
     4  Abhijat Malviya
     3  Bharath Vissapragada

$ git shortlog -s -n --all src/v/utils/
    68  Alexander Gallego
    58  Noah Watkins
    36  Michal Maslanka
    35  Duarte Nunes
    16  Evgeny Lazin
    14  Denis Rystsov
    14  Ben Pope
    10  Rob Blafford
     8  John Spray

$ git shortlog -s -n --all src/v/v8_engine/
     9  Vadim
     5  Michal Maslanka
     5  Vadim Plakhtinskiy
     3  Andrew Hsu

$ git shortlog -s -n --all src/v/cluster/rm_*
   129  Denis Rystsov
    11  Noah Watkins
     6  Alexey Zatelepin

$ git shortlog -s -n --all src/v/cluster/id_allocator*
    26  Denis Rystsov
     8  Michal Maslanka
     4  Alexey Zatelepin

$ git shortlog -s -n --all src/v/cluster/persisted_stm.*
    40  Denis Rystsov
    12  Alexey Zatelepin
     5  John Spray
     5  Michal Maslanka

$ git shortlog -s -n --all src/v/cluster/tm_stm.*
    65  Denis Rystsov
     4  Vadim Plakhtinskiy
     3  Alexey Zatelepin
     1  Andrew Hsu

$ git shortlog -s -n --all src/v/cluster/tx_*
    94  Denis Rystsov
     5  Noah Watkins
     4  Michal Maslanka
     2  Michał Maślanka

$ git shortlog -s -n --all src/v/cluster/tests/idempotency_tests.cc 
    17  Denis Rystsov
     2  Noah Watkins
     1  Andrew Hsu

$ git shortlog -s -n --all src/v/cluster/tests/id_allocator_stm_test.cc 
     7  Denis Rystsov
     1  Andrew Hsu

$ git shortlog -s -n --all src/v/cluster/tests/rm_stm_tests.cc 
    14  Denis Rystsov
     2  Noah Watkins
     1  Andrew Hsu

$ git shortlog -s -n --all src/v/cluster/tests/tm_stm_tests.cc 
    14  Denis Rystsov
     2  Noah Watkins
     1  Andrew Hsu

$ git shortlog -s -n --all src/v/kafka/server/rm_group_frontend*
    10  Denis Rystsov
     8  Noah Watkins
     2  Ben Pope

$ git shortlog -s -n --all src/v/kafka/**/*init_producer_id* 
     7  Noah Watkins
     1  Andrew Hsu
     1  Ben Pope

$ git shortlog -s -n --all  src/v/kafka/**/*txn*
     4  Denis Rystsov
     1  Andrew Hsu
     1  Ben Pope
     1  Noah Watkins

$ git shortlog -s -n --all  tests/
   552  John Spray
   433  Noah Watkins
   313  Michal Maslanka
   145  Ben Pope
   119  Rob Blafford
    79  Denis Rystsov
    75  Ivo Jimenez
    54  Vadim Plakhtinskiy
    50  Evgeny Lazin
    48  Michał Maślanka
    46  NyaliaLui
    36  Alexey Zatelepin

$ git shortlog -s -n --all tests/rptest/tests/compatibility
    13  NyaliaLui
     8  John Spray
     5  nyalia
     5  Noah Watkins
     2  Andrew Hsu

@mmedenjak mmedenjak added kind/enhance New feature or request and removed area/build labels Aug 9, 2022
@rishabh96b rishabh96b requested review from rishabh96b and removed request for gousteris August 9, 2022 13:31
@dotnwat dotnwat merged commit 6b713e4 into redpanda-data:dev Aug 9, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhance New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants