Pull Request Process

Development Developing for Plone
1

We have plenty of Pull Requests (PR) in Plone, 89 to this minute. Still to many! Also its difficut to get an overview of the state of an PR. I want to introduce a system of labels to manage this, as well as an process aka checklist.

After working some time through PRs I came up with the following process and states for labels. The labels would make it much easier to work with the list of open Plone PRs - I can remember a lot, but sometimes I have to open a PR again.

I propose a process as follows:

  1. Label PR
  2. Check if PR owner is Plone contributor. If not ask to become one.
  3. Needs rebase? If so request owner to do so.
  4. Quick review. Eventually delegate to package maintainers. Discuss, refine. Ask for the state.
  5. Start PR testing, either for 5.0 or 4.3
  6. If tests are green and review
  7. If discussion stalls for more than 2 month close the PR with a friendly message that it can be reopened if there is the plan to work further on this.

Labels are prefixed pr since github does not make a difference between issues and pr on this level:

  • pr p4.3, pr p5.0 - if its a plip the plip number and topic should be part of the title and a link in the body of the PR (white)

  • pr review needs a review or discussion is in progress (yellow)

  • pr wip work in progress, wait until finished (dark blue)

  • pr rebase needs a rebase (light blue)

  • pr testing testing is in progress (orange)

  • pr ok all ok, but for some reason like a dependency, release cycle merging is deferred (green)

2

+1.

It would be nice to have a nice overview over open PRs and somewhere in the docs a prominent link to that page.

3

The link to the overview is posted above.

4

I created the labels for all plone repositories (no, not manually, but with this script). Lets see how it works out.

I added also a pr orphaned for those PR where the original submitter is not available any more but the feature is very valuable.

5

All PRs are having labels now (manual work, sigh)

2 Likes
6

Excellent!!!

7

I've seen other projects (jenkins-ci and openstack at least) that add an automatic message when filling a pull request that basically tells them something along the lines of:

  • thanks for sending a pull request
  • someone will look at it briefly
  • here are (link) the usual process of dealing with pull requests on plone

So basically you make them aware of the process, we could even trigger jenkins pull request testing with this.

Now we need:

  • some webservice (jenkins, mr.roboto or a brand new thingie) that automatically sends this messages (I would say that maybe mr.roboto has nearly everything ready for it?)
  • the documentation that you just posted in docs.plone.org or somewhere else to be able to link to it

Great work btw!

8

I like this, it will make the whole thing more approachable for me. I hope to get some useful code into Plone soon, this gives me a clearer process to work with.

1 Like
9

These labels have shown to be really helpful. But I rarely set them myself and I am sometimes confused about their meaning.
I think, adding a verb to each label might help make it easier to grasp the meaning without referring to this page.
Here is my suggestions:

  • pr review needed
  • pr is wip
  • pr needs rebase
  • pr is under test
  • pr ok but deferred

What do you think?

I'd offer to write the script to update the labels (if this doesn't happen automatically)

10

+1 for human-understandable labels

11

indeed, youre right. i tried to keep it short. I have a script to rename labels, so if i find some time i'll ty to to my best to cleanup here.

1 Like