Why not deal with github pull requests efficiently ?

See: https://github.com/openwrt/openwrt/pulls

Can you explain?

你可以解释吗?

I'm pretty sure he's asking why time to review differs greatly between PRs.

1 Like

Why not deal with github pull requests efficiently?

Usually due to one of these:

  • Lack of manpower (commit touches part of the system nobody is familiar with)
  • Lack of interest (PR is too complex or trivial to attract a reviewer)
  • PR has various little issues which make it annoying to merge (some post processing is required, Sob missing or wrong, description not ideal, style quirks in code)
  • PR has unresolved discussion

I just tried to merge ten trivial random PRs, among these three required a rework, one had a broken commit message which I accidentally committed, one added unrelated stuff to the PR, one required a complete rewording of subject + message + exchange of Sob... overall this task took about half an hour for trivial changes to shell script oriented packages.

You can imagine that reviewing and testing complex PRs with hard to determine consequences (e.g. due to touching kernel, system wide components, versions) takes significantly longer, coupled with an overall rather low ratio of contributors actively merging PRs.

6 Likes

Hi xiaobo If you feel like you can help out pleas feel free. We need all the help we can get. thanks

1 Like

@all I see. Thank you for productive efforts, which will lead Openwrt in a good direction. Thank for everyone hard work.

1 Like

Not much a contributor can do to help, when important PRs go months or more without meaningful review from core developers that can commit. Eventually, the contributor gives up on rebase after rebase and then the complaint is "oh, it doesn't merge cleanly" so it rots.

Open over six months now:

Open over three months now:

Over seven months from submission to merge:

...and that's just mentioning a few :wink:

1 Like

So it appears that the amount of contributions exceeds the current review capabilities, question is how to solve that without lowering the standards.

There is some effort spent in improving the CI tooling which at least removes the compile test burden. Adding further people to the equation only got us so far - at least most PRs are now categorized and receive some amount of feedback, yet the amount of merges didn‘t increase proportionally.

Now there needs to be an automatism (as in rules/guidelines) to reach a verdict on open ones. From my experience, whenever we try to mass-merge PRs, a certain extent of unexpected fallout occurs, so people are hesitant to merge things they‘re not intimately involved in.

The only solution I personally see is to automate most aspects of PR review, to cover things like code style, buildability and rebase- or mergeability.

The remaining things like runtime regressions can be easily dealt with by radically reverting every PR which introduces issues - so in other words be liberal with merges but also with reverts.

Another thing I noticed is that nobody dares to simply reject PRs that have no chance to get merged, leading to the impression that everything that has been submitted will also end up in the tree eventually, which isn‘t the case - so this is something that requires some thoughts too.

Finally, and this is my own opinion, I don‘t think that ~170 PRs going back as far as one year is so extremely bad.

2 Likes

"deal with" essentially means merge or don't merge.

stagnation is the child of subjectivity. distribute that, and you have grey area soup.

so...

  1. enable alternate options for "dealing with"... aka... "master-stage-pending-pull" or other creative ways to facilitate testing, use and visibility of contributor codebase'

  2. assign a dev in monthly rotation as "priority-pull-request-liaison"... with the duties of interacting with core developers and contributors to resolve technical bottlenecks... ( not really "make your code look like this" or soft skills ... fundamental feedback, code contribution )

  3. if a way for "accurate community tagging/voting" ( edit: emphasis here on way.... contributor driven action ) of pull requests can be devised... tags like "device-addition", "core-feature", "feature-improvement" times X votes will prioritize fairly what is on top of the stack for the person assigned to "monthly-priority-liaison/feature progression duties" which would provide a much needed balance between developer and community focus.

I’m not a fan of “voting” to do much more than very, very roughly gauge interest. Most “critical” changes are not even visible to most end users. Ones around maintainability and extensibility don’t even directly impact end users.

I do agree that grooming the backlog (including that on patchwork) has value. This includes, in my opinion

  • Sorry, not going to happen, with a one-line explanation of why (example, “OpenWrt does not support devices without Ethernet at this time.”)
  • Your submission needs significant work, see (some URL on some submissions) items B, C, and F