How to speed up review

Description of the problem

Problem

Three.js is getting more popular and bigger. Lot of PRs are opened day after day.

I think it leads to a problem. Lately reviewing takes longer. Then many PRs are stacked now. On 2019/05/17 there’re 257 opened PRs. 65 of them are tagged as r105 (next release). Some of them are looking ok to me, or even approved by reviewers, but not merged yet.

Currently only @mrdoob makes the final decision to merge a PR basically. As Three.js is bigger he has a heavier workload for reviewing. And when he has some busy periods, the three.js development slows down.

Potential solutions

I’d like to discuss how we the contributors can help him and can speed up Three.js development.

Rough idea in my mind so far is, I think granting merge to some of the contributors and distributing workload may be good. For example

  • Grant merging obviously clear and correct PRs, for example just typo or easy bug fix, to some of contributors.
  • Grant merging PRs for a certain module to the module author or contributors who mainly maintenance

And also @mrdoob can grant merge to assigned reviewers.

With these ideas, he can focus on only the PRs very depending on his opinion.

Any thoughts?

And @mrdoob, if you have any ideas to help you, I’d very pleased if you share with us. I’m sure many developers including me are willing to help you.

Maybe related threads : #14263 #11318

Three.js version
  • Dev
  • r104
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, …)

Author: Fantashit

7 thoughts on “How to speed up review

  1. Thanks for sharing your concerns Takahiro.

    For the last few months I got sidetracked with model-viewer and it left me very little time or energy to review PRs here. Google I/O became a milestone for the project and now that we passed it (and fixed some mroe lingering issues) I should be able to redirect my focus to three.js again.

    Lets see how the next few weeks go. If you feel things do now improve, let me/us know here and we can investigate further.

  2. Additionally, I’ve been considering adopting https://opencollective.com/ so we could fund contributors.

    I do not think bounties are a good approach. Instead, I wonder how people would feel about donating monthly then we can use that to give active contributors a salary.

    I’m personally already being funded by Google to continue my work helping people doing cool sites 🙌

  3. I have one more suggestion. Instead of having more people who can merge, it could be possible to have a bit more screening in place, to signal to Mr Doob that some patches are better than others. This can be something like “oh, hey, look, this pull request has 2 approvals by Xyz and Whosit”

  4. Besides, even if these PRs are approved, we should go step by step in order to ensure robustness. It took us three releases (R103, R104 and R105) to remove all bugs after the parameters renderTarget and forceClear were removed from WebGLRenderer.render() in R102. Software changes (especially in the core) can have a lot of unexpected side effects…

  5. On 2019/05/17 there’re 257 opened PRs.

    On 2019/09/24 we reduced the number of open PRs to 154. Making progress 😊

Comments are closed.