[WIP] doc: add MAINTAINERS.md #26868

ghost commented at 3:54 AM on January 11, 2023: none

Picking up #25839 (labeled "Up for grabs")

Addressed all the comments from previous pull request, made a few other changes and added @JeremyRubin as co-author.

DrahtBot commented at 3:54 AM on January 11, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

in MAINTAINERS.md:35 in c7b5d4b135 outdated

  30 | +However, it is acceptable for maintainers to merge their own changes if pull request
  31 | +has enough reviews and no other maintainer is available to merge.
  32 | +
  33 | +## Nomination and Selection of Maintainers
  34 | +
  35 | +The current maintainers or contributors can open a 'call for maintainers' issue.

maflcko commented at 8:20 AM on January 11, 2023:

I don't think a single maintainer in the past has been picked based on such a thread, so it seems weird to start prescribing it now and mentioning it seems misleading.

unknown commented at 10:53 AM on January 11, 2023:

Removed 'call for maintainers' section and replaced by nomination in IRC channel. However, it was suggested by @ariard as an improvement that I agree with and could be discussed in next IRC meeting. It can be added later if enough contributors agree with it.

maflcko approved

maflcko commented at 8:20 AM on January 11, 2023: member

Please make sure the CI lint passes

in MAINTAINERS.md:19 in c7b5d4b135 outdated

  14 | +
  15 | +## Responsibilities and roles
  16 | +
  17 | +Project maintainers have commit access and are responsible for merging patches 
  18 | +from contributors. They perform a janitorial role merging patches that the team 
  19 | +agrees should be merged. They also act as a final check to ensure that patches

maflcko commented at 8:39 AM on January 11, 2023:

A maintainer decision isn't final. It is merely a piece before post-merge review and post-merge testing happens. Also, there is a release process and the every user is free to use their own judgement whether and what software they want to run.

unknown commented at 10:53 AM on January 11, 2023:

I copied this part from https://bitcoincore.org/en/about/. Rephrased 'final check' and added a sentence based on your suggestion.

michaelfolkson commented at 9:39 AM on January 11, 2023: none

I don't really see the point of this. What is this doc trying to achieve? What real world problems have you identified that this document provides a solution for?

If I'm forced I'll open an alternative PR with some wording on what maintainers do to nip this in the bud but it basically boils down to maintainers merge pull requests, a long term contributor can put their name forward to be a maintainer in a IRC meeting and open a pull request like Gloria, Vasil have done. It isn't going to be a particularly interesting or informative doc.

On the Vasil issue I think we're in agreement(?) @1440000bytes but I don't know how for example this document would resolve the Vasil issue in any way. Two maintainers are refusing to comment (or choosing to ignore the PR). Hence this document is pretty useless in solving the Vasil issue. If someone wants to put their name forward to be a maintainer they can in the IRC meeting. If they are a long term contributor they will know about the IRC meeting already and will be able to assess whether putting their name forward is appropriate or not. They won't need to refer to a document like this.

ghost commented at 10:54 AM on January 11, 2023: none

I don't really see the point of this. What is this doc trying to achieve? What real world problems have you identified that this document provides a solution for?

This document contains list of present maintainers, their role and selection process. Goal is to document things (related to maintainers for bitcoin core) that everyone agrees upon and improve with time.

This isn't a new practice and you might find lot of open source projects with such file if you search maintainers.md github in a search engine. There is a blog post about trying to solve related issues and start with maintainers.md.

I'll quote one line from it:

"MAINTAINERS.md is the first step towards finding human solutions to human problems"

If I'm forced I'll open an alternative PR with some wording on what maintainers do to nip this in the bud but it basically boils down to maintainers merge pull requests, a long term contributor can put their name forward to be a maintainer in a IRC meeting and open a pull request like Gloria, Vasil have done. It isn't going to be a particularly interesting or informative doc.

You are free to open another pull request.

#25839 was closed on Oct 3, 2022 and labelled 'up for grabs' since then. I picked it up based on a suggestion by @MarcoFalke in a comment.

On the Vasil issue I think we're in agreement(?) @1440000bytes but I don't know how for example this document would resolve the Vasil issue in any way. Two maintainers are refusing to comment (or choosing to ignore the PR). Hence this document is pretty useless in solving the Vasil issue. If someone wants to put their name forward to be a maintainer they can in the IRC meeting. If they are a long term contributor they will know about the IRC meeting already and will be able to assess whether putting their name forward is appropriate or not. They won't need to refer to a document like this.

I have added both pull requests as topic for next weekly bitcoin core IRC meeting. This document is public information which helps every new contributor to understand the process. I don't see any downsides of being transparent and document basic things.

michaelfolkson commented at 11:03 AM on January 11, 2023: none

This document contains list of present maintainers, their role and selection process. Goal is to document things (related to maintainers for bitcoin core) that everyone agrees upon and improve with time.

A list of present maintainers and their scope in a doc can't hurt. You keep using strange wording though. It isn't a "selection process" is it? Someone puts their name forward, it is discussed at a IRC meeting, they open a pull request. It seems like you keep wanting to shoehorn this open source project into a formal company structure with a board of directors, an election process and all this other stuff that is totally unnecessary.

You are free to open another pull request.

Ok I consider myself forced.

ghost commented at 11:32 AM on January 11, 2023: none

Please make sure the CI lint passes

Fixed

unknown marked this as ready for review on Jan 11, 2023

michaelfolkson commented at 9:06 PM on January 12, 2023: none

Ok I consider myself forced.

I won't be opening an alternative PR. This PR can be reviewed as is.

in MAINTAINERS.md:43 in a3aac6f3b5 outdated

  38 | +role of a maintainer. Contributors are free to self nominate as well.
  39 | +
  40 | +Maintainers are added by a new maintainer opening a pull-request to add their
  41 | +key to the Trusted Keys file. For example, see [this pull
  42 | +request](https://github.com/bitcoin/bitcoin/pull/23798). The adding of the key
  43 | +is reviewed by the community and merged by one of the maintainer. Before opening

vnprc commented at 2:14 AM on January 13, 2023:

Typo: "maintainer" should be "maintainers"

unknown commented at 12:32 PM on January 13, 2023:

Fixed

ghost commented at 12:41 PM on January 13, 2023: none

A list of present maintainers and their scope in a doc can't hurt. You keep using strange wording though. It isn't a "selection process" is it? @michaelfolkson Replaced 'select' with 'add'

English isn't my first language. Feel free to suggest more improvements if you see any.

in MAINTAINERS.md:5 in c76446d6d0 outdated

   0 | @@ -0,0 +1,59 @@
   1 | +# Maintainers
   2 | +
   3 | +This document contains list of present maintainers, their role and process to add new maintainers.
   4 | +
   5 | +## Current Maintainers

jonatack commented at 4:18 PM on January 13, 2023:

I'm unsure if this section is a good idea, though perhaps not for a good reason. In general, people (e.g. journalists and non-contributors to the project) in my experience overestimate the role of maintainers (i.e. as power/control/gatekeepers rather than implementing the rough consensus of reviewers), and conversely, they undervalue or may be unaware of the key role of the contributors doing consistent review and testing that shape and determine rough consensus.

Also, these roles may be fluid, require updating here, and "General" before each seems redundant, so would drop the section.

unknown commented at 6:34 PM on January 13, 2023:

Removed 'Focus Area' column
I understand your point about journalists however they would anyway find these names from other links which maybe misleading, outdated etc. Role of maintainers and contributors is defined in next section with 'rough consensus' part mentioned. A maintainers.md file without names of maintainers would be incomplete IMO

in MAINTAINERS.md:22 in c76446d6d0 outdated

  17 | +Project maintainers have commit access and are responsible for merging patches
  18 | +from contributors. They perform a janitorial role merging patches that the team
  19 | +agrees should be merged. Maintainers make sure that patches are safe and align
  20 | +with the project goals. However, their decision is not the last step, as patches
  21 | +must also go through post-merge review and testing before being implemented.
  22 | +The maintainers’ role is by agreement of project contributors.

jonatack commented at 4:21 PM on January 13, 2023:

The maintainers’ role is by rough consensus of project contributors.

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:19 in c76446d6d0 outdated

  14 | +
  15 | +## Responsibilities and roles
  16 | +
  17 | +Project maintainers have commit access and are responsible for merging patches
  18 | +from contributors. They perform a janitorial role merging patches that the team
  19 | +agrees should be merged. Maintainers make sure that patches are safe and align

jonatack commented at 4:26 PM on January 13, 2023:

perhaps: "They perform a janitorial role of implementing, by merging patches, the rough consensus of reviewers. Maintainers also make sure..."

unknown commented at 6:33 PM on January 13, 2023:

I used this sentence, let me know if it conveys the correct meaning:

"They perform a janitorial role of implementing the rough consensus of reviewers by merging patches."

in MAINTAINERS.md:31 in c76446d6d0 outdated

  26 | +In general, it is considered best practice for maintainers to avoid merging their
  27 | +own changes directly into the codebase. This is because merging your own code can
  28 | +create a potential conflict of interest, where a maintainer's personal interests
  29 | +may override the best interests of the project.
  30 | +
  31 | +However, it is acceptable for maintainers to merge their own changes if pull request

jonatack commented at 4:28 PM on January 13, 2023:

However, it may be acceptable for maintainers to occasionally merge their own changes if the pull request

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:34 in c76446d6d0 outdated

  29 | +may override the best interests of the project.
  30 | +
  31 | +However, it is acceptable for maintainers to merge their own changes if pull request
  32 | +has enough reviews and no other maintainer is available to merge.
  33 | +
  34 | +## Nomination and Adding New Maintainers

jonatack commented at 4:30 PM on January 13, 2023:

## Nomination and Addition of New Maintainers

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:36 in c76446d6d0 outdated

  31 | +However, it is acceptable for maintainers to merge their own changes if pull request
  32 | +has enough reviews and no other maintainer is available to merge.
  33 | +
  34 | +## Nomination and Adding New Maintainers
  35 | +
  36 | +The current maintainers or contributors can nominate a long term contributor

jonatack commented at 4:31 PM on January 13, 2023:

I think the word "contributor" includes maintainers.

Contributors can nominate a long-term contributor

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:38 in c76446d6d0 outdated

  33 | +
  34 | +## Nomination and Adding New Maintainers
  35 | +
  36 | +The current maintainers or contributors can nominate a long term contributor
  37 | +in the [IRC channel](https://web.libera.chat/#bitcoin-core-dev) to take on the
  38 | +role of a maintainer. Contributors are free to self nominate as well.

jonatack commented at 4:35 PM on January 13, 2023:

role of a maintainer. Contributors are free to self-nominate as well.

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:40 in c76446d6d0 outdated

  35 | +
  36 | +The current maintainers or contributors can nominate a long term contributor
  37 | +in the [IRC channel](https://web.libera.chat/#bitcoin-core-dev) to take on the
  38 | +role of a maintainer. Contributors are free to self nominate as well.
  39 | +
  40 | +Maintainers are added by a new maintainer opening a pull-request to add their

jonatack commented at 4:35 PM on January 13, 2023:

Maintainers are added by a new maintainer opening a pull request to add their

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:46 in c76446d6d0 outdated

  41 | +key to the Trusted Keys file. For example, see [this pull
  42 | +request](https://github.com/bitcoin/bitcoin/pull/23798). The adding of the key
  43 | +is reviewed by the community and merged by one of the maintainers. Before opening
  44 | +such a pull request, it is typical that the adding
  45 | +of the maintainer might be discussed in the regular Bitcoin Core IRC meeting,
  46 | +but the actual decision making process occurs in the pull request.

jonatack commented at 4:36 PM on January 13, 2023:

but the actual decision-making process occurs in the pull request.

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:43 in c76446d6d0 outdated

  38 | +role of a maintainer. Contributors are free to self nominate as well.
  39 | +
  40 | +Maintainers are added by a new maintainer opening a pull-request to add their
  41 | +key to the Trusted Keys file. For example, see [this pull
  42 | +request](https://github.com/bitcoin/bitcoin/pull/23798). The adding of the key
  43 | +is reviewed by the community and merged by one of the maintainers. Before opening

jonatack commented at 4:42 PM on January 13, 2023:

is reviewed by the community and merged by one of the maintainers based on rough consensus of the reviewing contributors. Before opening

unknown commented at 6:33 PM on January 13, 2023:

Done

in MAINTAINERS.md:58 in c76446d6d0 outdated

  53 | +
  54 | +## Maintainer Removal
  55 | +
  56 | +There is currently no established procedure for removing a maintainer. If it is
  57 | +deemed necessary to remove a maintainer, it should be discussed openly among the
  58 | +other maintainers and contributors and a decision should be made through a

jonatack commented at 4:51 PM on January 13, 2023:

Perhaps this sentence would suffice for this paragraph: "If it is deemed necessary to remove a maintainer, the decision should be made by open discussion and rough consensus of contributors and the community."

unknown commented at 6:33 PM on January 13, 2023:

Done

in CONTRIBUTING.md:12 in c76446d6d0 outdated

  11 | @@ -12,7 +12,8 @@ revolves around a meritocracy where contributors earn trust from the developer
  12 |  community over time. Nevertheless, some hierarchy is necessary for practical

jonatack commented at 4:52 PM on January 13, 2023:

community over time. Nevertheless, some roles are necessary for practical

unknown commented at 6:33 PM on January 13, 2023:

Done

jonatack commented at 4:53 PM on January 13, 2023: member

A few suggestions from which to pick and choose.

add MAINTAINERS.md

Co-Authored-By: Jeremy Rubin <886523+JeremyRubin@users.noreply.github.com>

e9734c9fd2

unknown closed this on Feb 22, 2023

unknown deleted the branch on Feb 22, 2023

bitcoin locked this on Apr 16, 2025