Tldr: Write a maintainer's guide

[waldyrious]

We need to have a well-defined (public) procedure for handling PRs, since there's a chronic issue of a PR backlog we can't seem to get rid of entirely. It could be something like:

• PRs will be merged once they (1) pass the automated tests (Travis CI and CLA signing) without issues, (2) have the review comments addressed (3) get the thumbs-up of two maintainers -- the second one can do the merging immediately after accepting.
• If a week goes by after (1) and (2) are satisfied, but no second maintainer has manifested their approval, the first maintainer can merge.
• If a month goes by without reaction from the submitter (e.g. to fix lint errors or review comments), the PR should be taken over by a maintainer who should ensure that at least the page conforms to the contribution guidelines and passes Travis CI before merging.
  
  
  
  • Here, "taking over" means making any necessary changes, including rebase, reword of commit messages, and change of content, while preserving the authorship metadata (e.g. by amending commits rather than copy-pasting the content)
  
  

This should ensure we remain responsive to contributions, and that contributors have a clear notion of what to expect (and when to call out maintainers if they fail to adhere :P)

Any thoughts? Did I miss something?

[agnivade]

Agree on all points. :100:

I do have some concerns on the 3rd point though. I had spent quite some time merging old PRs. And it seems like the backlog is building up again. Now, I just don't feel like doing the whole exercise again. :disappointed: And imposing the task of making the necessary changes(which might include reading up the man page and studying the command) to the maintainers is a bit unfair I believe.

I absolutely want to remain responsive to contributions so that everyone feels like they can contribute. But, when a contributor does not respond even after pinging separately, it becomes a bit difficult to go through the task of merging the PR time and again.

[waldyrious]

We can leave it at the discretion of the maintainers: the explicit guidelines would make it clear to everyone when the maintainers could override a PR's author without stepping on any toes, but that would be a right, not a duty. This way, if merging the PR would entail too much work, it would be ok to simply close the PR with a note to that effect. Sounds good?

[agnivade]

Again, "too much work" is a subjective thing. But yea, I am leaning towards the fact that its okay for us to close a PR, after pinging the contributor and waiting for one week after no response.

[waldyrious]

Sure, the ideia is that either resolution would be acceptable (and predictable), so maintainers would have the option to choose whatever they deem appropriate given the specifics of the situation.

[agnivade]

Great. Where do you think we should put this ?

[waldyrious]

I'm thinking of adding this to a GOVERNANCE.md page (something quite simple and free-form -- just a list of the guidelines for maintainership and handling of issues and PRs that we've been following so far). This would also serve as a great place to implement #1117.

Before going ahead, I'd like to hear someone else's opinion -- at least @ostera's.

[waldyrious]

In the meantime, @agnivade, do you think we could repurpose @tldr-bot to (also) perform the necessary pinging after the periods we define here expire? You've been doing that manually, but we should automate as much of this activity as possible, so as to free up contributor/maintainer time for the actual changes :)

The bot could even close PRs as you did in #975 -- the wording you used there seems like a great template we could adopt: friendly, informative, and with clear action paths for alternative resolution.

[agnivade]

That sounds like a nice idea for a new project. I don't think there are any tools currently which do that(need to check though).

However, we still have to resolve the original problem of tldr-bot (the issue of it not getting the token for PRs coming from a different repo). Getting a separate server for ourselves seems to be the only way to go.

[waldyrious]

Sure. I guess @ostera also could have something to say here :)

I wonder if there's any host for server-side services that offers a free plan for FOSS projects, though, like Travis and others do for the CI side of things. Any ideas?

[agnivade]

We could use heroku, but their free tier is too poor. Don't know about any other services.

[jeeftor]

Could ya'll also add more maintainers? How many of you guys are actually active?

[agnivade]

Could ya'll also add more maintainers?

Yea absolutely. I have been thinking of adding new ppl. More thoughts on that here - https://github.com/tldr-pages/tldr/issues/1117.

How many of you guys are actually active?

Me and @waldyrious usually are. @ostera also chimes in from time to time.

Will you be interested in joining as a member ? :wink: Don't hesitate to hop on to our gitter channel if you have more questions.

[agnivade]

@waldyrious - Now that you are back, would you like to take care of this -

I'm thinking of adding this to a GOVERNANCE.md page (something quite simple and free-form -- just a list of the guidelines for maintainership and handling of issues and PRs that we've been following so far). This would also serve as a great place to implement #1117.

This has been quite some time pending, and pretty important now that we have a new collaborator(@jeeftor).

[waldyrious]

@waldyrious - Now that you are back

Slowly doing so -- there's a big backlog to process, but I'll get it done :)

would you like to take care of this

just the other day I submitted such a document to another project I help maintain, and I'm planning on submitting something quite similar for tldr-pages. I will probably prioritize handling the open PRs first, but I've set a reminder to come back to this once that backlog is at least down to a manageable size (in terms of my involvement).

[waldyrious]

As a note to self: some of the things we'll want in that document are:

• a guideline that all trivial fixes (typos, syntax fixes, etc.) are to be made by the maintainers
• an explicit mention of our principles of enabling web-based contributions as much as possible (including offering to perform rebases or other complex git operations on behalf of contributors, or helping them if they want to do it themselves).

We'll also want to get useful recommendations from https://opensource.guide and maintainerati.

[waldyrious]

For future reference, here's a link to a summary I made for @sbrl in the project's Gitter chat room. Quoted below for convenience:

• We generally prefer to have at least two maintainers review each PR, so for example if one reviews and approves it, the other can merge the PR once they agree as well (unless someone else raises objections).
  If other maintainers are too busy and a PR lingers around for too long without a second maintainer’s approval, they can be merged with a single maintainer’s approval, in the interests of providing a timely response to contributions
• Generally we try to avoid merge commits, to allow a cleaner git history. If the commits in the PR are worth keeping separately (i.e. they represent semantic changes rather than typo fixes or syntax corrections revealed by TravisCI, have descriptive commit messages, etc.), then the "rebase and merge” option is the preferred one.
  Otherwise, if they’re just a bunch of commits named “edited command.md” or similar, then the "squash and merge” option is to be used instead.
• For simple, uncontroversial fixes and typos to PR files, it’s best if the maintainer does them rather than ask the contributor, especially because the actual content-related discussion may end up requiring quite a bit of changes
  In such cases, it’s preferable to either use “rebase and merge” option on the github web interface, or an interactive rebase in the comand line, to clean up any commits that would need to be squashed, reworded, etc.

[waldyrious]

From https://github.com/tldr-pages/tldr/pull/1407#issuecomment-308665847:

when merging PRs by squashing, it's generally recommended to edit the commit message body and remove any superfluous details, or those that only make sense within the sequence of changes made during the PR (in effect, ensuring the whole thing is written as if it was a single commit from the start).

For PRs that introduce new pages, it would suffice to use a single-line commit message (without a body) saying "\

[waldyrious]

From https://github.com/tldr-pages/tldr/pull/1429#issuecomment-324032251:

[whether to edit PRs by other users] is generally subjective, but the rule of thumb I've been using is a mix of three criteria:
1) how many changes we've already requested from the author (not individual commits, but review rounds separated by new commits they make), and whether they're showing signs of fatigue (less enthusiastic responses, slower reactions)
2) having all commits in a PR by the same author, ideally, so they can be squashed or rebaseed cleanly at the end of the review process.
3) how subjective is the change, e.g. uncontroversial stuff like typos or formatting to match the project guidelines vs. rewordings, reorderings, etc.

I.e., on the one hand we don't want to be overly burdensome for contributors, and thus should do stuff ourselves when possible, but on the other hand we do want to give full credit to contributors (and have a clean git history) when they did pretty much all of the work. There isn't a single answer; it's definitely a subtle balance we must handle.

[waldyrious]

@agnivade and @sbrl: now that #1117 was fixed via #1839, which included a brand new maintainers-guide.md, my next goal is to complete it to close this issue.

Can you review that document and the comments above, and let me know if there's anything that isn't covered?

[agnivade]

I believe these points were not covered ?

We generally prefer to have at least two maintainers review each PR, so for example if one reviews and approves it, the other can merge the PR once they agree as well (unless someone else raises objections).
If other maintainers are too busy and a PR lingers around for too long without a second maintainer’s approval, they can be merged with a single maintainer’s approval, in the interests of providing a timely response to contributions

[waldyrious]

I believe these points were not covered ?

Sorry, I wasn't clear. I meant covered by the combination of the current document + the comments above. Meaning that I'll complete that document to include everything mentioned above, plus anything else you guys think is missing.

[sbrl]

Right! Yeah, I think everything is either covered by the comments here or the new maintainers guide 😺

[agnivade]

Oops :sweat_smile: Yep, everything is covered. I dont have anything to add.

[waldyrious]

Cool! I'll get to work on this as soon as I can. For self-reference, Homebrew's invitation message for new maintainers has some useful notes at the end.

[waldyrious]

Oh, one more thing that just occurred to me: maintainers are expected to hang around in the Gitter chat room on a regular basis, or at least show up every now and then.

[agnivade]

Good point, let's add that.

[sbrl]

Ah, yeah. I have it open as a pinned tab on my home laptop.