Contribution and merge policy

[warpfork]

I'd like to increase the broadness and openness of the contribution and the merge policy in this repo.

I don't know how exactly we're going to do this :dancers: but let's figure something out.

Goals

The goal is that more people should be able to review and to contribute code to this repo. Reviews should be fast; merges (if applicable) should be fast. If things aren't fast, it should at least be because there's discussion, rather than because there's not enough people paying enough attention (which is sometimes currently the case).

People should have more than one option for who to "tap on the shoulder" when they would like to contribute code and need reviews.

And not to put too fine a point on it, but, yes, I want to shift this to not always involve me as the final word. :)

Non-goals

Mind, the goal is not necessarily to shift this repo towards being an "everything and the kitchen sink" scene. Code here should still be fairly "core" (whatever that means) and relatively minimal "batteries included" stuff. It's still perfectly fine and good for new features, certainly new codecs, etc, to be written in other repos. In fact, in many cases, it should still be encouraged -- and we can upstream features to this repo when they're proven to work (or when lessons were learned from their first implementation!), and if we decide they're "core". This issue isn't really focusing on how we determine that; it's more focused on who is the most involved in determining it :)

A nongoal, at least for now, is to try to specialize subpackages of the repo as having different maintainers. It may be a good idea in the future! But it seems like a bit much to bite off, and not yet entirely necessary, today.

Discuss!

I'll post my first proposal below. This was already raised in matrix/discord chat a while back, so there are already some discussion notes from there, and from community calls, that I'll add. (Any transcription errors mine!)

I (or someone else with edit power!) can edit this initial post later with a pointer to any resolutions.

[warpfork]

A proposal of a +1/+2 system

(First seen in chat, here: https://matrix.to/#/!RUdkFuMSfKYSOvwuFe:ipfs.io/$oNMiZ4iLmQXIoglKdjbY588BBi2RGx3tKhKeMPgs6FI?via=ipfs.io&via=matrix.org )

I propose that to make it clear that other people can move things along, we adopt a "+2/+1" system. (I haven't used this before, but it seems to sort of intuitively make sense, and gives at least a little structure.)

I propose @mvdan and myself get +2 power. (This is defacto how we're already treating it. It's also the people most likely to be the most deeply on the hook for maintaining whatever does get merged, and the folks who are most stuck saddled in the seat of having to think about API consistency.) Maybe also @rvagg here if he doesn't resist 😈

I propose some folks like @willscott, @hannahhoward, @gammazero, @masih get +1 power, as established contributors who also have a deeply practical idea of how to contribute. Maybe several more names belong here as well.

And the general rule is: To merge: Anything that gets a total of +2 can be merged. You can +1 your own things. (You can use +2 power on yourself, too, if you've got it... Although if you've got +2 power in the first place, it's assumed you're going to be judicious about doing so.)

So to work some examples, this would mean:

• Usually if (e.g.) mvdan pushes code, he's still gonna wait for another +1 review.
• ...but if he's really confident, and I'm out, and he can't find anyone else with the spare time, I definitely want him to feel free to proceed.
• If (e.g.) hannahhoward or willscott (people with +1 power) want to merge something, and they can get each other to review it and agree it's shipshape: they're each contributing a +1, so that's a +2, and they're clear to merge.
• If there's an outside contributor: mvdan or I can look at it, give it a +2, and merge. Or, two other folks with +1 can look at it (and feel free to tap each other on the shoulder), and if they both give it a thumbs up, that's a green light too.

This may need tuning. For example: I think the +1 list could easily be longer, but also may benefit from having a somewhat bounded length -- not because of low trust, but just to make sure information still has a reasonably high chance of "fanning in"; also to keep it so that if the load were round-robin'd, people would stay relatively "warm" on the domain rather than feel yanked from a distance. But I don't really have a strong feeling on what's appropriate here, so feedback on details like this highly welcome.

---

(There have already been a bunch of comments on this -- I'll replay them in separated posts below.)

[warpfork]

@willscott earlier commented:

is it worth using a CODEOWNERS file or similar mechanical support?

i think codeowners has semantics - 1 username / line - it will enforce an approval by someone on the list, with repo admins being able to dismiss that requirement if they wish. that, along with comment blocks for '+2's is probably a reasonable 'in-code' documentation of this policy

Answer: Potentially. Not yet decided. Mechanical support can be useful. But we don't have a ton of existing hands-on familiarity with that particular mechanism, so we'd want to consider it further before leaping for it. It's also unclear how much we really need this to be mechanical, vs having this structure as a series of social nudges that are mostly about encouraging people to feel empowered by a structure (vs feeling unempowered by a lack of structure).

[warpfork]

@rvagg commented in a community call:

• for comparison, some brief highlights on the node community norms:
  • you have to leave things up at least a day, ideally more, a business day -- around the world
  • you can get +1's and -1's -- what's important is that you don't get any -1's.
  • getting a minus-one means a discussion starts.
  • reverts, even after something is in master, follow the same process -- it's very possible.
  • this process in Node works with 100s of contributors and keeps things quite lively!
  • does tend to put some onus on release process -- someone owning releases is supposed to do a final pass on the state of things, and open revert requests if they're uncomfortable.
  • as a last ditch: there is a "technical committee" that can force resolutions.
• notes that the above does exist in the context of having release cycles, and porting it to a scenario without such cycles might be different.
• notes that a bunch of original Node contributors had a "small core" philosophy (prefer doing things in userland) -- but this didn't really get transcribed or enshrined and there's neither cultural nor procedural pushback anymore.
• thinks there might be some overhead to this +2 thing.

[warpfork]

various folks discussed the use of github "review comments" feature during a community call:

• problem: using them creates some impression that re-review needs to come from the same person. This is problematic because it's often not true, but the github UI/UX for the feature creates this appearance anyway.
• problem: doesn't communicate nuance like "it would be ideal if X (but nonblocking)"

There were many other remarks on this system that I was not fast enough to transcribe. Holistically, people are just not impressed with the github review comments system at all.

[warpfork]

in matrix/discord, mark5891 comments (original: https://matrix.to/#/!RUdkFuMSfKYSOvwuFe:ipfs.io/$8Cn1mzKnmixqoa8ECdXnd7d0EpZ32-YuPh9yelRRQSk?via=ipfs.io&via=matrix.org ):

The gist of it: there will be nuance: accept this; documenting a general direction and allowing it to be organic seems reasonable; "Don't try to document every exception. Some common sense goes a long way"

Some excerpts:

a bit of a nuance to be made. I for example - with 0 IPLD knowledge - can have a look and say "ahh, looks fine to me. I +1 it". That might be a totally useless +1. Here too in other open source projects it is "common practice" that people who give +1 do at least have a bit of history in the project. So total newcommers giving a +1 should be regarded as noise, their +1 is worth nothing. But once those newcommers have build a bit of credibility with for example code reviews, commits, or other contributions then you can count the +1. Again though, this is quite a "hands on" approach. There isn't really a golden rule for this.

Don't try to document every exception. Some common sense goes a long way

[warpfork]

Many discussions remark on a balance: If barriers to contribution are too high, contributors may just leave and implement their feature in their own repos.

The interesting thing about this balance, of course, is the further question: is this good or bad? :)

If contribution frictions keep new code out of the core, it means the core doesn't grow. That could be bad. On the other hand: a core that doesn't grow can be good :)

More features in core:

• :+1: those features are more discoverable
• :+1: those features are less likely to be re-implemented by uncoordinated folks spending unnecessarily repeated work
• :-1: the core gets harder and harder to maintain as it gets bigger
• :-1: contributors may walk away after contributing, leaving more to maintain placed on fewer shoulders
• :waffle: if other higher-level features want to build on a feature, if it's in core, that's "easy"; if it's out of core, dependency trees get "deeper" and that tends to correlate with a more fracture-prone ecosystem. (Intense handwaving here, but you can't tell me it isn't true.)
• :-1: features placed in core necessarily take up some amount of namespace -- this is a precious resource, and may prevent other features in the same region from being developed
  • :-1::-1::-1: if this happens in a topic where there is more than one viable solution and we don't actually know which one is preferable yet (or if there's one strictly superior mechanism at all)
• :+1: if there are major changes in the core, then updates get done all at once in the whole core (in practice: CI forces everything to work, all the time, and doesn't let changes go out at all until that's solved). This means changes can be made and propagated rapidly (...aside from effects that may be had on downstreams out of the repo); good for innovation within the area.

Features get shipped somewhere else:

• :-1: those features may be less discoverable.
  • At the very least, it eventually becomes important to build curated indexes of "you may be interested in..."
• :+1: the core isn't getting harder to maintain!
• :-1: the same feature may get implemented more than once by different folks, if they failed to discover each other's work early.
• :+1: the same feature may get implemented more than once by different folks, and one of them might do it better!
  • :+1::+1::+1: for features kept outside of the core, there are no barriers to this kind of innovation!
• :+1::+1::+1: there are zero coordination frictions; the developer develops to whatever cadence and quality bar they desire.
• :waffle: the feature might be subject to less rigor and demand for test fixtures. Maybe the developer emphasizes this; maybe they don't; there's no organizational control.
  • :-1: if this feature turns into a major one which folks in the community need to reproduce in multiple implementations, and there turns out to be no spec, or an "implementation defined spec", and no easily portable test fixtures... then a lot of people get very sad in the long run.
• :-1: if there are major changes in the core, then updates need to be made downstream too -- and sometimes that's not easy, or involves multiple people. At some point, this can begin to slow down innovation rates across an ecosystem.

In summary: there are advantages to moving some code into a "core" position (e.g. this repo), but there are also advantages to producing code in other repos rather than moving it to the core (whether immediately, or sometimes, at all). It's situational! It's contextual! The world is complex!

[warpfork]

I myself want to note loudly that I'm still going to harp on things being developed with language-agnostic data-driven test fixtures. That is my :sparkles: thing :sparkles: this year -- my biggest goal and wish is more fixtures, more compatibility checklists, and more implementations of IPLD systems in not-golang.

There are many reasons for this:

I think data-driven test fixtures are the highest-leverage thing we can do to ensure IPLD has a long life and grows in multiple languages and ecosystems.

I think reviewing serial fixtures is drastically easier and more reliable than reviewing code. It's also accessible to more people since anyone in the ecosystem can review a serial fixture, rather than just people fluent in one programming language.

I think serial fixtures are powerful documentation. Some of the most useful examples and documentation we have to date aren't API docs; they're examples of data-in data-out. I observe that when linking folks to documentation, often, our fixtures pages in the site are the most useful!

I think serial fixtures are a powerful design focuser. They make sure we know what a declarative, data-driven way to use a feature would be -- and that's almost infinitely valuable, considering the core ethos of our whole project to manifest a decentralized data-first world. Anecdotally, I've found features I write with a language-agnostic data-driven fixture system in hand come out designed better than features that start in pure golang and glue on a serial adapter later; they also usually get developed faster because I worry about the incidentals of golang ergonomics less and the core algorithm and behavior correspondingly more.

All ~three~ four of these reasons alone are powerful, but with all ~three~ four of them taken together...! I think it's generally speaking a mistake to launch new developments without driving them with serial fixtures.

But, all that said -- if someone wants to merge stuff in some implementation without that drag, and they can gather the plusses to merge it from other contributors who are comfortable without requiring such tests, then, indeed, per this policy: I'm only going to ask. (But from me? You'll probably never get a +2 from me for something that comes in without fixtures!)

[warpfork]

I've had one additional thought that might be worth appending to this, as a result of some other conversations today:

Perhaps +2ers (or perhaps even +1ers) can also nominate someone to be a +1er for a specific PR.

Someone: anyone. Even someone who's a new contributor.

The purpose of this would be twofold: 1) Give people a neat hands-on opportunity to get their feet wet with the interaction process! 2) Give us a way to drag domain experts in on some topic, and explicitly describe and acknowledge the value of their expertise!

[warpfork]

I'd like to note that this whole process (or any process we could possibly imagine, I expect) is enough of a mouthful, and needs to become known to enough people, that we're going to end up repeating it.

(And that's okay.)

For example, I did a brief situationally-focused reminder and rundown of how this process would work in a comment on a PR here: https://github.com/ipld/go-ipld-prime/pull/358#issuecomment-1055461893 .

I think this might be worth hoisting into some sort of a playbook. It seems like it's going to be naturally recurring.

(The alternative is: if there's a process, but people don't know about it, and don't have some contextual guidance and reminders about it, then... well, broadly, a process no one knows about might as well not exist. And in the lack of a process, the default for most folks and personalities seems to be to fall all the way back to feeling need for total consensus if they don’t feel total ownership. That fallback to consensus is something we definitely want to avoid; it's almost guaranteed to produce gridlocks and painfully slow inaction. So: overcommunicate. Repeat as necessary.)

(It would be neat if this could be automated entirely, but I don't know if I'm bullish on that. If we take it as presumed that we live within github rather than some other review tool, the options for automating this information distribution is... limited. Bots are possible, but I don't think they tend to have good holistic outcomes, personally.)

[RangerMauve]

Regarding exposing the process to people, I think it'd be useful to add a block to PR templates which contains this information so that it can be discoverable.

[BigLep]

2022-05-03 conversation: the merge policy has simplified currently due to the very low resources on the team and low number of contributions.

We'll keep the issue open, but we're not taking on getting it into a .md yet.