Skip to content

Commit

Permalink
Merge #18949: doc: Add CODEOWNERS file to automatically nominate PR r…
Browse files Browse the repository at this point in the history
…eviewers

a06eb03 doc: Add comments and additional reviewers to CODEOWNERS file (Adam Jonas)
e02da22 doc: Add CODEOWNERS file (Wladimir J. van der Laan)

Pull request description:

  This PR brings back and builds on #17094. GitHub uses a CODEOWNERS magic file to automatically add tagged contributors to the "Reviewers" list for a PR.

  The goal of this is to make use of GitHub's suggested reviewers feature and not to confer ownership or give veto power to specific people. It would be better if this file could be named CODEREVIEWERS, but alas, that wouldn't work. The idea of a NAGFILE was proposed in [Bitcoin Core Dev meeting in 2018](https://diyhpl.us/wiki/transcripts/bitcoin-core-dev-tech/2018-03-07-priorities/#:~:text=NAGFILE). While this GitHub implementation has some complications, it's a step towards realizing the promise of automating "reviewing begging" and (hopefully) positively impacting the review process as a whole.

  Of secondary value, this file can serve as documentation for who the maintainers are and who it might be smart to check with for certain areas of code/features (i.e., fuzzing, PSBT, and Bech32) -- this is helpful information for new contributors.

  * The first commit is taken from #17094
  * The second commit adds comments and expands the list of reviewers based on the suggestions and comments from that PR
  * ~The third WIP commit~ This commit also uses the doc dir as an example of granular assignments based on lines of codes ~contributed~ written and/or general engagement with the project. (If interested, here is a report for [most lines of code per author for each file](https://gist.github.com/adamjonas/854a46a1918224927b186865baeac411)). The pro of this level of detail is that the best reviewer is more likely to be nominated. The con is that it will create churn as files are renamed, new files are added, or reviewers want to be added or removed.

  Some open questions:
  * How often should this file be changed?
  * What level of history does one need have on the project before being added to this file? When does it make sense to remove a reviewer?
  * These review notifications can [cause a lot of noise](https://git.luolix.topmunity/t5/How-to-use-Git-and-GitHub/Team-based-notifications-or-rework-CODEOWNERS-notification/td-p/7811) and automatically subscribes the requested reviewer to the thread. A GitHub Team based approach would allow for adding or removing reviewers without modifying this file; however, this comes along with its [own set of problems](https://bionic.fullstory.com/taming-github-codeowners-with-bots/#problems-with-github-code-owners), including granting [write access](https://git.luolix.topmunity/t5/How-to-use-Git-and-GitHub/CODEOWNERS-works-with-users-but-not-teams/td-p/4986#U4991). Other projects [have used bots](https://bionic.fullstory.com/taming-github-codeowners-with-bots/#using-a-github-bot) to sidestep this.

Top commit has no ACKs.

Tree-SHA512: aa674ac62478b8801f48750df869c802070dc83d0fa9ff93596e9d63406129d7fd3c0daeb35d7a1a259554d045c24746a6808878a7b9867c7ed66d251f0c918f
  • Loading branch information
MarcoFalke committed Sep 20, 2020
2 parents 831b0ec + a06eb03 commit 38fd1bd
Showing 1 changed file with 136 additions and 0 deletions.
136 changes: 136 additions & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# ==============================================================================
# Bitcoin Core CODEOWNERS
# ==============================================================================

# Configuration of code ownership and review approvals for the bitcoin/bitcoin
# repo.

# Order is important; the last matching pattern takes the most precedence.
# More info on how this file works can be found at:
# https://help.github.com/articles/about-codeowners/

# This file is called CODEOWNERS because it is a magic file for GitHub to
# automatically suggest reviewers. In this project's case, the names below
# should be thought of as code reviewers rather than owners. Regular
# contributors are free to add their names to specific directories or files
# provided that they are willing to provide a review when automatically
# assigned.

# Absence from this list should not be interpreted as a discouragement to
# review a pull request. Peer review is always welcome and is a critical
# component of the progress of the codebase. Information on peer review
# guidelines can be found in the CONTRIBUTING.md doc.


# Maintainers
# @laanwj
# @sipa
# @fanquake
# @jonasschnelli
# @marcofalke
# @meshcollider

# Docs
/doc/*[a-zA-Z-].md @harding
/doc/Doxyfile.in @fanquake
/doc/REST-interface.md @jonasschnelli
/doc/benchmarking.md @ariard
/doc/bitcoin-conf.md @hebasto
/doc/build-freebsd.md @fanquake
/doc/build-netbsd.md @fanquake
/doc/build-openbsd.md @laanwj
/doc/build-osx.md @fanquake
/doc/build-unix.md @laanwj
/doc/build-windows.md @sipsorcery
/doc/dependencies.md @fanquake
/doc/developer-notes.md @laanwj
/doc/files.md @hebasto
/doc/gitian-building.md @laanwj
/doc/reduce-memory.md @fanquake
/doc/reduce-traffic.md @jonasschnelli
/doc/release-process.md @laanwj
/doc/translation_strings_policy.md @laanwj

# Build aux
/build-aux/m4/bitcoin_qt.m4 @hebasto

# MSVC build system
/build_msvc/ @sipsorcery

# Settings
/src/util/settings.* @ryanofsky

# Fuzzing
/src/test/fuzz/ @practicalswift
/doc/fuzzing.md @practicalswift

# Test framework
/test/functional/mempool_updatefromblock.py @hebasto
/test/functional/feature_asmap.py @jonatack
/test/functional/interface_bitcoin_cli.py @jonatack
/test/functional/tool_wallet.py @jonatack

# Translations
/src/util/translation.h @hebasto

# Dev Tools
/contrib/devtools/security-check.py @fanquake
/contrib/devtools/test-security-check.py @fanquake
/contrib/devtools/symbol-check.py @fanquake

# Gitian/Guix
/contrib/gitian-build.py @hebasto
/contrib/guix/ @dongcarl

# Compatibility
/src/compat/glibc_* @fanquake

# GUI
/src/qt/forms/ @hebasto

# Wallet
/src/wallet/ @achow101

# CLI
/src/bitcoin-cli.cpp @jonatack

# Coinstats
/src/node/coinstats.* @fjahr

# Index
/src/index/ @fjahr

# Descriptors
*descriptor* @achow101 @sipa

# Interfaces
/src/interfaces/ @ryanofsky

# DB
/src/txdb.* @jamesob
/src/dbwrapper.* @jamesob

# Scripts/Linter
*.sh @practicalswift
/test/lint/ @practicalswift
/test/lint/lint-shell.sh @hebasto

# Bech32
/src/bech32.* @sipa
/src/bench/bech32.* @sipa

# PSBT
/src/psbt* @achow101
/src/node/psbt* @achow101
/doc/psbt.md @achow101

# P2P
/src/net_processing.* @sipa
/src/protocol.* @sipa

# Consensus
/src/coins.* @sipa @jamesob
/src/script/script.* @sipa
/src/script/interpreter.* @sipa
/src/validation.* @sipa
/src/consensus/ @sipa

0 comments on commit 38fd1bd

Please sign in to comment.