RFC: marvin-mk2 the pull request bot

Hi!

I have finally gotten around to writing a simple PR workflow bot. Say hello to marvin!

Here is an example of how it works: Test PR by timokau · Pull Request #13 · timokau/marvins-playground · GitHub

The intention is basically to start by replacing the “PRs ready for review” and “PRs already reviewed” threads on discourse. It could learn to do lots more than that – but let’s start simple and iterate from there. See [RFC 0030] Formalize review workflow by timokau · Pull Request #30 · NixOS/rfcs · GitHub for more details.

The bot in its current state is very simple. Its a proof of concept. It is not designed to handle every eventuality or have every feature. Currently it crashes on any unexpected situation. I haven’t made it more robust on purpose, since failing early is good for now.

I would like to run a alpha test on nixpkgs. To do that, we’d need to give marvin triage permission. The absolut worst-case would be that it mangles the metadata a bit, though I think its very unlikely. It is currently strictly opt-in and has straightforward code to bail out early if its not invited on a PR.

After that, the plan is to iterate on it until I’m reasonably happy with it and its well tested. At that point I would revive rfc#30, which would propose enabling it by default on every PR.

What do you think? Feel free to play around with it.

11 Likes

A little update: Marvin is now a Github App and runs on nix-community infrastructure!

Since GitHub Apps can’t be mentioned, I’ll have to change the command syntax (I’m thinking /status needs_review?). After that add some robustness (no longer crashing on every unforseen event) and then it should be good for a real alpha run.

3 Likes

I consider the minimum-viable-product done now. Intended behaviour is as follows:

  • only act on PRs where the PR author has opted in by writing /marvin opt-in (in a separate line)
  • print an explanatory message
  • allow anybody to switch between the needs_review, needs_work and needs_merge state with /state needs_review etc.

If you want to do any testing, now is the time. Try to break it. Give feedback. Keep in mind that it is supposed to be a minimal viable product though.

It runs on GitHub - timokau/marvins-playground: Playground for marvin-mk2. Do whatever you want there.

The code is here: GitHub - timokau/marvin-mk2: Discontinued! See https://github.com/timokau/marvin-mk2/issues/34#issuecomment-1100656280 (Previously: "Making sure your PR gets a review and your reviews don't get lost.")

Its deployed :tada:

If you want to test it on your PR, opt-in with /marvin opt-in. Commands have to be in their own line. The bot will explain itself.

If you want to help out and review PRs, here’s the queue of PRs ready for review: Issues · NixOS/nixpkgs · GitHub

If you have merge permission, here are the already reviewed PRs: Issues · NixOS/nixpkgs · GitHub

If a PR is not actionable for you as a reviewer, you should change its state. If no state fits a PR, create an issue at the marvin-mk2 repo. For example we’ll probably need a waiting_for_upstream at some point).

3 Likes