OCA Github Bot

[jgrandguillaume]

Goal

• Develop a bot that will help the review process for the maintainer, help newcomers, close too old PRs, etc..

Current troubles

• Time to merge, # of PR increases
  • → Generates frustration among new contributors
  • → Discouraging contributors
• Travis is not always green
  • Lots of time taken to make Travis green
  • Not easy for newcomers to make it green
• Merging rules might be too strict ?
• PSC might have the right to merge faster? Yes, PSC can decide if merge something although some guidelines are not met.
• Optimistic Merging (OM) instead of Pessimistic Merging (PM)?
• Difference between new modules and existing modules ?
• Different maturity levels in the same projects/repo in term of quality, reliability, usage
• Different level of maintenance among them (some are very active, others abandoned)
• Different levels of responsibility from the PSC

Wanted Features

Version 1

• Tag pull requests with "New contributor" for people with less than 3 PR within the OCA repos
• Close and comment ("Please re-open if necessary") every PR older than 6 months without comments
• Tag and comment if the pull request is ready to be merged (2 approvals, 5 days, green CI)

Later

• Tag “new module proposal” or “improvements on existing”
• Post message in case of:
  • Travis red -> how to solve
  • Newcomers -> Say hello, thanks you, where to look for help + pointer to doc or relevant link

Resources

Use : https://github.com/facebook/mention-bot
Note this module already interact with github and odoo https://github.com/legalsylvain/oca-custom

[pedrobaeza]

Add also this one:

• Announce that the PR is ready to be merged if there are 2 approvals, not current comments and there are more than 5 days from the initial request.

[hbrunn]

yes, @pedrobaeza's is a very good one. Even better would be if the bot set a label for that, then we can filter for that and merge with just a glance on the comments if there's anything blocking

[pedrobaeza]

Yeah, tagging + mentioning should be the perfect combination!

[lasley]

Close and comment ("Please re-open if necessary") every PR older than 6 months without comments

Isn't this maybe a bit aggressive? I know I have a bunch of PRs that are good to go and are just waiting for review by someone with time. I feel like if we start closing these types of PRs, we'll end up losing good code.

Maybe some sort of ping before closing?

[jcdrubay]

Close and comment ("Please re-open if necessary") every PR older than 6 months without comments

How about:

• One notifications that it will be closed automatically in 7 days
• When closing automatically, add a label Closed by bot

One more idea for the features

• OCA Karma points: Calculate an "OCA Karma" to promote biggest contributors based on commits, reviews, comments, triage, issue report. Today, people promoted are the one posting apps (open or closed) in apps.odoo, but the one in contributing to one of of the many repositories of OCA don't get much attention. That's mainly because there is no "Cross repository graph", graphs are only at repository level.

[elicoidal]

@pedrobaeza

Announce that the PR is ready to be merged if there are 2 approvals, not current comments and there are more than 5 days from the initial request.

How is it different from @jgrandguillaume proposal here:

Tag and comment if the pull request is ready to be merged (2 approvals, 5 days, green CI)

[Garamotte]

@elicoidal Maybe @jgrandguillaume edited after @pedrobaeza's comment :)

In the actual troubles, we have a rare case, but that might be annoying : OCA/server-tools#689
In short : CI green on the PR, CI green upstream, but CI red when PR is merged with upstream because of some changes applied in the meantime.
If the bot could check this before tagging the PR as mergeable (or comment instead), this would be great (maybe to be added later).

[eLBati]

Isn't this maybe a bit aggressive? I know I have a bunch of PRs that are good to go and are just waiting for review by someone with time. I feel like if we start closing these types of PRs, we'll end up losing good code

@lasley @jgrandguillaume I think the automatic closure should only happen for PRs not tagged as needs review, that is PRs that need fixing or are WIP or help wanted etc
We should automatically close PRs that nobody is interested in

[lasley]

I think the automatic closure should only happen for PRs not tagged as needs review

Yeah that makes sense, and I'm good with that.

An interesting nice-to-have on the note of tags would be a way to have the bot add/remove tags with some sort of comment command.

This would be helpful for contributors that know the tag rules, but don't actually have write access to the repos & even more useful for the PSCs that randomly get pinged just to manage a tag. Admittedly it's usually not me that gets pinged, but I think I see @pedrobaeza's pain on that.

[lasley]

#310 supports my previous comment for the need of tags. Is this a v1 feature or a later?

[zoek1]

Is this still open for contribution? @jgrandguillaume

[jgrandguillaume]

@zoek1 Yes ! Sorry for late answer. I think it has been started here: #346

[max3903]

Closing to allow the bounty claim.