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

[RFC] Project governance, Maintainer's guide #1839

Merged
merged 13 commits into from
Jan 9, 2018
Merged

[RFC] Project governance, Maintainer's guide #1839

merged 13 commits into from
Jan 9, 2018

Conversation

waldyrious
Copy link
Member

@waldyrious waldyrious commented Dec 27, 2017

Finally!!! 🎉 🎉 After months of slow progress, I was finally able to make the final stretch and complete these documents that hopefully reflect what is already the spirit of the tldr-pages project. I'm really happy with how they turned out! 😄

Of course, feedback from the entire community is welcome.

Note that, as a reflection of the notes herein, @sbrl should be made an owner of the tldr-pages org -- which is long overdue IMO!

These guidelines will also invite discussion about the status of the current members of the maintenance team who haven't been active in the past few months. The pertinence of these changes is up for discussion, of course. For example, the inactivity period may be extended, or the entire concept rejected. Let me know what you think.

Follow-up of #1694. Closes #1117, and partially addresses #1209 (further details will be needed later, as described in that thread).

@waldyrious
Copy link
Member Author

@waldyrious waldyrious added decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. documentation Issues/PRs modifying the documentation. labels Dec 27, 2017
@felixonmars
Copy link
Collaborator

Thanks for the work! Does contributing to clients under the tldr-pages org count in the PRs for role transition? There are several references to "the project" but this remains unclear to me :)

Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

I think this missed out on the at-least 2 approvals per PR rule ?

And also, after this PR gets merged, should we do the due diligence and make @sbrl as owner and deactivate others who have not been active for more than 6 months ?

GOVERNANCE.md Outdated
in the [MAINTAINERS.md](MAINTAINERS.md) file.

- Specifically: once a contributor has been an organization member for at least 3 months,
and has **reviewed or merged 10 pull requests** by other contributors,
Copy link
Member

Choose a reason for hiding this comment

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

I would rather this be merged 10 pull requests and reviewed 5 pull requests. I want to keep a minimum bar on the reviewing part.

Copy link
Member Author

@waldyrious waldyrious Dec 27, 2017

Choose a reason for hiding this comment

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

Fine by me 👍. I'll wait for others' comments before making changes.

Copy link

Choose a reason for hiding this comment

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

Probably worth including some hint at needing some substance to the review, otherwise you'll be drowned in "LGTM" comments, and newcomers will be surprised when maintainers disagree with the "LGTM" littered on their PRs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I guess phrasing it as "non-trivial reviews" will suffice, but let me know if you have other ideas.

Copy link
Member

Choose a reason for hiding this comment

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

Good points here!

- Ideally, **every new discussion should receive a response within 24 hours**.
You can respond yourself or ask other members to provide their thoughts/opinions.

- When mergin PRs, use the strategy that produces a **clean git history** in the repository:
Copy link
Member

Choose a reason for hiding this comment

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

typo at mergin

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops 😄 will fix.

@agnivade
Copy link
Member

@felixonmars -

Does contributing to clients under the tldr-pages org count in the PRs for role transition?

No, I would not want that. Contributing code is very different that contributing pages. if someone contributes regularly to a client, they need to be made a member of that repo. This guide is just for tldr pages.

@waldyrious
Copy link
Member Author

I think this missed out on the at-least 2 approvals per PR rule ?

Yes, not only that, but several other details. I didn't want to delay this further by attempting to make it perfect the first time. That's what I'm talking about in the opening comment when I wrote the following:

partially addresses #1209 (further details will be needed later, as described in that thread).

So this won't close that issue, but it's a start :)

@waldyrious
Copy link
Member Author

And also, after this PR gets merged, should we do the due diligence and make @sbrl as owner and deactivate others who have not been active for more than 6 months ?

Yes, that was my thinking. I wrote about that in the opening comment:

Note that, as a reflection of the notes herein, @sbrl should be made an owner of the tldr-pages org -- which is long overdue IMO!

These guidelines will also invite discussion about the status of the current members of the maintenance team who haven't been active in the past few months. The pertinence of these changes is up for discussion, of course. For example, the inactivity period may be extended, or the entire concept rejected. Let me know what you think.

@waldyrious
Copy link
Member Author

waldyrious commented Dec 27, 2017

Does contributing to clients under the tldr-pages org count in the PRs for role transition?

No, I would not want that. Contributing code is very different that contributing pages. if someone contributes regularly to a client, they need to be made a member of that repo. This guide is just for tldr pages.

I actually don't think it's that clear-cut. On the one hand, yes, this guide is meant to help keep a healthy maintenance team for the tldr pages content; on the other hand, there are other aspects of the ecosystem that undeniably contribute to its success, not the least of which are the clients (but also the linter, the website, etc.). Another point to consider is that being added to the organization does give access to all the repos within it, not just the pages repo. So for consistency, we could argue that development work in the non-content repos within the org should qualify for regular membership in the org, and maintenance work in the non-content repos within the org should qualify for ownership in the org. That would make this entire thing more symmetric and simpler to explain, too.

That said, I still believe that moving the clients out of the umbrella of the org would be ideal, so those repos would automatically stop qualifying at that time. Does this solution (i.e. these guidelines covering all repos in the org, and moving the client repos out of the org) make sense for you guys, @felixonmars and @agnivade?

Also, let me reiterate that membership status within the org is not meant as privilege or recognition, but simply as a reflection of the actual state of the community -- so that both newcomers and regulars can have an accurate picture of the project's health and size of the effective maintenance team.

@agnivade
Copy link
Member

Another point to consider is that being added to the organization does give access to all the repos within it, not just the pages repo.

Right, I hadn't thought of that. In that case, let us start by adding the person as a collaborator to the pages repo (and not as a member of the org) once the first milestone is achieved. We can add a second milestone, which can be like 10 PRs merged, 10 PRs reviewed after which a person becomes an org member. And then, if a person is active for more than 6 months upgrade him/her to an owner.

That said, I still believe that moving the clients out of the umbrella of the org would be ideal, so those repos would automatically stop qualifying at that time.

If we implement my idea above, this won't be necessary. Even if we don't consider the issue of membership at all, I think it's just easier to keep them under the same org. Though it's not a strong negative from me.

@waldyrious
Copy link
Member Author

In that case, let us start by adding the person as a collaborator to the pages repo (and not as a member of the org) once the first milestone is achieved. We can add a second milestone, which can be like 10 PRs merged, 10 PRs reviewed after which a person becomes an org member. And then, if a person is active for more than 6 months upgrade him/her to an owner.

Yeah, that's one of the options I had considered. The only reason I didn't include the extra level was to make the guidelines as simple and inclusive as possible, and avoid too much bureaucracy (after all, the goal here is to make the transitions frictionless and as automatic as possible). But if the community prefers the 3 levels you're proposing, I'd be fine with that too.

GOVERNANCE.md Outdated
This means they will be able to add people to the organization,
manage all the organization's repositories, configure integrations, etc.

- If a collaborator or maintainer stops being active in the project for more than 6 months,
Copy link

Choose a reason for hiding this comment

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

maybe state 'calendar months', so that people can easily look at their monthly activity on GitHub to determine when they need to do some more work in order to keep 'active'. Ideally somehow phrase it so that an action on Jan 1 requires another action on July 31 at the latest.

Also I see one listed maintainer who this would apply to the moment the document becomes effective, in which case it would be nice to offer them the chance to fix the problem, either by doing an action, or stepping down so they are not listed in the maintainers document only to be immediately.

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the sentiment, but I'm not sure "calendar months" would make the language clearer. For me at least, it would only make me more confused, as that's not a term I'm familiar with as a non-native English speaker.

In any case, the idea here is not to define activity requirements that people would have to fulfill in order to keep their membership. It's more of a way to allow the official list of maintainers to adjust to accurately reflect the actual maintenance team. Perhaps we could tweak the wording here to reinforce this sentiment?

Also, note that since becoming active again is sufficient to have the membership reinstated (as the initial thresholds had already been met the first time around), this shouldn't be a hassle for contributors who go through a period of reduced availability. If anything, it should make things easier for them, by making it official that they're not publicly identified as maintainers, as that can convey a sense of obligation -- which we want to avoid because it can quickly lead to burnout.

Copy link

@jayvdb jayvdb Dec 27, 2017

Choose a reason for hiding this comment

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

"more than 6 months" means "6 months and one day"

"more than 6 calendar months" means anywhere between 6 and 7 complete months (minus seconds) depending on when the first action occurred.

The intend is being able to use GitHub's monthly activity list to see which month the person was active and let them have six complete named months pass without activity before the pin is pull, without caring which day of the month their last activity was.

And moving people to inactive would be a regular first day of month maintenance task, not relative to when each persons last action day occurred.

Copy link
Member Author

Choose a reason for hiding this comment

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

use GitHub's monthly activity list to see which month the person was active

Are you talking about the activity graph in the user profile? Perhaps this wasn't clear, but the intention here was to count activity in the tldr project, not on github overall. Or am I missing something?

And moving people to inactive would be a regular first day of month maintenance task, not relative to when each persons last action day occurred.

That's a good point, but the idea was not to control each maintainer's activity down to such detail, but just to provide a rough indication of the period after which it would make sense to look at how long they've been effectively inactive in the project and then update the member list accordingly. If it takes a week or a month more to update the list, there's no harm. Besides, in our experience this shouldn't be a common enough occurrence to warrant establishing a monthly cleanup schedule.

That said, I think we can add an additional bullet point saying that they should get a friendly message informing them of the fact -- perhaps even provide a boilerplate message similar to the one Homebrew provides for inviting maintainers, just to make sure that the removal won't give out the wrong impression.

Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Awesome work, @waldyrious! This PR should help out a ton. Perhaps even a link to this under contributing in the README would be helpful?

Also, ping @jsonbruce (as I'm aware you were wondering about this kind of thing a little while ago).

GOVERNANCE.md Outdated
in the [MAINTAINERS.md](MAINTAINERS.md) file.

- Specifically: once a contributor has been an organization member for at least 3 months,
and has **reviewed or merged 10 pull requests** by other contributors,
Copy link
Member

Choose a reason for hiding this comment

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

Good points here!

is exposed to maintainers, who can then identify and address bottlenecks or inconveniences.
Similarly, **avoid merging your own PRs**.

- Ideally, **every new discussion should receive a response within 24 hours**.
Copy link
Member

Choose a reason for hiding this comment

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

This is quite tight. I've been working to ~48hrs up until now. Do I need to change this?

Copy link
Member Author

@waldyrious waldyrious Dec 27, 2017

Choose a reason for hiding this comment

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

Yeah, the 24h was just a number that sounded reasonable on the surface when I wrote this. I didn't put too much thought into it because I assumed the "ideally" would suffice to make it clear that it's in no way an obligation, just a nice-to-have. But I can see how it could be interpreted as some sort of a deadline.

I'm perfectly fine with changing it to 48h, 3 days, one week, or whatever you guys feel is reasonable for both the contributors and the maintainers.

Copy link
Member

Choose a reason for hiding this comment

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

I am good with "ideally". Being an open-source project, I believe, by definition it does not have a deadline.

Copy link
Member

Choose a reason for hiding this comment

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

@agnivade true....? Perhaps it could not be bold then? That would make it less imposing.

@waldyrious
Copy link
Member Author

Perhaps even a link to this under contributing in the README would be helpful?

Excellent point, it didn't occur to me. I'll add a commit adding such links later today.

@waldyrious
Copy link
Member Author

Just pushed a new commit adding a link to the governance policy in the README.md and CONTRIBUTING.md.

@maxsxu
Copy link
Collaborator

maxsxu commented Dec 28, 2017

Thanks for notification! @sbrl

I have to say, this awesome project is really attractive to me, helps me a lot, and in line with my concept.
I'm very expected to join this organization and do more contributions. I'm not sure if anybody can send me an invitation.

With the growth of information, I believe that thetldr will become more and more popular!

@agnivade
Copy link
Member

@jsonbruce - After this PR gets approved and merged, you will be eligible for the collaborator position.

@waldyrious
Copy link
Member Author

waldyrious commented Dec 28, 2017

To recap the discussion so far, I'm looking forward to the following additional inputs:

  1. @jayvdb's response here and here.
  2. @sbrl's response here. @agnivade seems to favor the current wording, with 24h + ideally (please confirm), while I was convinced by @sbrl's comment that a wider period (perhaps 3 days?) may sound less imposing.
  3. everyone's thoughts on whether to add a third level, as suggested by @agnivade here. Despite it adding yet another step in the progression, I admit that the cleaner split in the metrics (contributions for level 1, maintenance work for level 2, and length of activity period for level 3) is elegant enough to balance it.
  4. general comments from the maintainers who have yet to chime in (which may take a few days, given the time of the year).

@agnivade
Copy link
Member

@agnivade seems to favor the current wording, with 24h + ideally (please confirm)

Confirmed.

GOVERNANCE.md Outdated
- Members of the organization who demonstrate interest in performing maintainership tasks,
by reviewing and/or merging PRs, responding to and labeling issues, and generally doing project maintenance work,
shall be made part of the maintenance team, and their name added to the list of current maintainers
in the [MAINTAINERS.md](MAINTAINERS.md) file.
Copy link

Choose a reason for hiding this comment

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

I recommend to recognize non code contributions as well. We've had people being active on our code of conduct, performing usability tests or just "joking around making the community more happy".

Copy link
Member Author

@waldyrious waldyrious Dec 30, 2017

Choose a reason for hiding this comment

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

@sils fully agreed. I'm not sure where you're getting the "code-only contributions" vibe from, though. Can you quote the specific passages that you'd suggest rephrasing?

Or was this meant as a general comment to the whole document? Note that the very first bullet point ("All contributions are welcome") already makes this clarification, even linking to the https://github.com/kentcdodds/all-contributors repo.

Copy link

Choose a reason for hiding this comment

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

you seem to be measuring activity by PRs made or reviewed. I think we have an additional clause that is vague by design and allows the maintainers to rank up anyone for arbitrary awesome stuff done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see what you mean. While this text explicitly mentions a variety of maintenance-related activities:

reviewing and/or merging PRs, responding to and labeling issues, and generally doing project maintenance work

...the actual metrics only take PRs merged and reviewed into account. The first reason for this is that those are clear, transparent metrics that are easy to check; the second, and most important reason IMO, is to avoid giving these role transitions a meaning of recognition of past work, i.e. make them into some sort of promotion/demotion. Instead, they should occur in a casual, natural manner, in either direction, without connotations of evaluation or judgment (positive or negative). For example, nobody has to second-guess why person X was added and person Y wasn't, nor why person Z was removed. I think you'll agree that it's very hard to measure "awesomeness" objectively :) and in a way that everyone will agree.

That said, such a vague clause is already possible, via the "all decisions are made by community consensus" item. As a concrete example, I was planning on starting a discussion, after this PR, to propose making @rprieto, as the project's creator, not affected by the inactivity guidelines.

That way we allow this document to remain a set of loose guidelines, rather than an exhaustive list of procedures. Adding exceptions for all edge cases would make it increasingly more verbose and, ironically, less flexible.

† In fact, ideally these role transitions would happen automatically, as is the case for example with StackOverflow, where privileges are distributed automatically based on karma, and even the moderators are elected by the entire community.

GOVERNANCE.md Outdated
or in the [Gitter chat room](https://gitter.im/tldr-pages/tldr)
(which is open to all, and publicly logged).

- **All decisions are made by community consensus.**
Copy link
Contributor

@leostera leostera Dec 30, 2017

Choose a reason for hiding this comment

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

So after reading about it in the link you have below, it looks like the word we want to use is consent rather than consensus here.

So rather than by community consensus, it'd read with community consent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was a little liberal with the terminology here, but the fuzziness is intentional (hence the "ideally" below). The rationale for this choice of words is (1) I assumed most people would be more likely to get the right idea from "community consensus" than "community consent", and (2) I wanted to give the community some wiggle room, rather than hint to a strict specification of the consent method as a rule, since it can be abused as some sort of veto, which can cause friction and bottlenecks in discussions.

Does this make sense to you?

@leostera
Copy link
Contributor

Besides that small comment, and a request to reformat this to be terminal friendly (80 columns), I think this is an astounding piece of writing that we very very much need. 👏

@agnivade
Copy link
Member

agnivade commented Jan 6, 2018

@waldyrious - It looks like there are no further comments. Do you want to update the PR with the proposed changes ?

@waldyrious
Copy link
Member Author

waldyrious commented Jan 9, 2018

I've pushed a few commits that hopefully address all the points raised:

And of course, I'd like you all to take a look at the rendered document. Let me know what you think!

Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

Amazing work @waldyrious ! Fantastically written :)

Long time pending. Feels great to close this.

@waldyrious waldyrious force-pushed the governance branch 2 times, most recently from 433ffca to 1796e6e Compare January 9, 2018 16:49
@waldyrious
Copy link
Member Author

waldyrious commented Jan 9, 2018

Thanks, @agnivade, it means a lot coming from you :)

ps - force-pushed to fix a couple typos that had slipped in. Edited my previous comment to update the commit hashes.

Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Looks ok to me too! Thanks for the great work, everyone 😺

@sbrl sbrl merged commit 7fac214 into master Jan 9, 2018
@sbrl
Copy link
Member

sbrl commented Jan 9, 2018

Performed regular merge as the commit messages were useful, and two separate maintainers approved this PR.

@agnivade agnivade deleted the governance branch January 9, 2018 18:56
@waldyrious
Copy link
Member Author

waldyrious commented Jan 9, 2018

Thanks all for the careful review and helpful comments! I'll open a new issue to track the implementation of some of the changes now approved.

@agnivade
Copy link
Member

@sbrl - You could do a rebase if you want to preserve the commit messages.

@waldyrious
Copy link
Member Author

I've left a comment on #1887 about rebase vs. merge. Thanks for opening that, by the way, because I don't believe I had formulated that reasoning explicitly before, and now there's a written record :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues/PRs dealing with role changes and community organization. decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. documentation Issues/PRs modifying the documentation.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants