On average it takes about two weeks for a PR to get merged on theforeman/foreman, but that doesn’t mean that every PR is merged within that time, for a few it take two weeks or even longer till they have a first review/comment.
While there are many reasons why a PR takes time to merge, it should be important to give and get feedback as early as possible. For this reason I wanted to start a discussion on this topic, to see what can be done to improve.
There is already a GitHub template in theforeman/foreman when opening a pull request, which provides hints for requirements of a PR to submit. This could be extended with a few headlines and suggestions of information to include to make it easier for the PR to be tested and reviewed, like including rough steps to test and reproduce the issues/fix/feature. Or add hints what to add, that could increase the likelyhood of a PR being reviewed.
I think the best way to answer the question what can be done to increase the likelyhood of a PR being reviewed is to ask “What keeps you from reviewing?”
I don’t mind having PRs untouched for few weeks, that happens and eventually when you poke people they will start a review. But what’s driving me crazy is when someone starts a review, I need to rebase the PR several times and then the PR gets lost and reviewer never returns back. I mean, a PR can be in non-mergeable state that’s fine but it must be obvious from reviewer comments because then both sides can wait forever.
I usually skip PRs where I see someone is making comments because this makes me think that someone is looking into the PR. This is lack of github feature - ability to see if a PR has an assigned reviewer in notifications without hovering (it shows it after mouse hover tho but with amount of PRs I don’t do that and make a wild guess).
We have a label called “Not yet reviewed” and I think this label is IMHO useless, the bot removes it the moment a PR is merged, so it’s a label that is present while a PR is opened. That’s zero information and actually misleading one - reviewers rarely remove this flag. At least I forget to do this all the time. We should be looking at “Review required” built-in flag only.
Reviving an old but still relevant thread I meant to reply on but clearly forgot.
When you use the Github review feature, the bot should remove this automatically. I have seen that it doesn’t always, but if you look at https://github.com/theforeman/foreman/pulls you can see that most don’t have it and the bot removed it.
We do have the Needs re-review. This is automatically added to a PR when the author pushes a new commit (whether it’s fast forward or force). It also clears the Waiting on contributor label.
When you do a review and set it to Request changes it automatically adds the Waiting on contributor label.
