Hi,
On Tue, Jul 20, 2021 at 11:41:37AM -0700, Ben Pfaff wrote: > The OVS review process has greatly slowed over the last few years. This > is partly because I haven't been able to spend as much time on review, > since I was once the most productive reviewer. Ilya has been able to > step up the amount of review he does, but that still isn't enough to > keep up with the workload. > > We need to come up with some way to improve things. Here are a few > ideas, mostly from a call earlier today (that was mainly about the OVS > conference). I hope they will prompt a discussion. > > * Since patches are coming in, we have people who are knowledgable about > the code. Those people should be pitching in with reviews as well. > It doesn't seem like they or their managers have the right incentives > to do that. Maybe there is some way to improve the incentives. There is that and also that even with others review, a maintainer still does a careful review which puts off reviewers' work. "The perfect is the enemy of the good." We have several emeritus maintainers but only a couple of them active at the moment. It doesn't seem fair to ask them to do more work. So, I agree with the other reply to this thread saying that we need more maintainers. Although I think we could grow the number of maintainers, it doesn't seem reasonable to ask each one of them to do an extensive review by themselves on every patch. My point is that there should be a "chain of trust", or a rule that allows a patch to be "blindly committed" if that patch is reviewed by N different members, for example. The intention is not to forbid maintainers to review, but to alleviate the load or pressure on them when the community already helped. As a side effect, more people could become maintainers/committers because we don't require them to know OVS deeply or to commit huge amounts of time to careful review others work. This brings up another point: Unpredictability. It is not possible today to tell what is going to happen with a patch after it gets posted to mailing list. It can be silent ignored for many months, or be reviewed by others and still not accepted, etc. We should have a way to prevent that otherwise it makes very difficult to align companies or other upstream projects schedules. For example, if a company is investing on a feature "X" most probably has a deliver date. Even if the work is posted during development phase, that doesn't mean the patch will be reviewed or accepted or rejected. It's kind of chicken and egg issue. If the process is not clear, why managers should provide more incentives to participate? > * The Linux kernel uses something like a "default accept" policy for > patches that superficially look good and compile and boot, if there is > no review from a specific maintainer within a few days. The patches > do get some small amount of review from a subsystem maintainer. OVS > could adopt a similar policy. I agree. Aaron is working to improve patchwork's status report for each patch. Hopefully each community member could report test status there. It seems that if the community decides to go with the "default accept" rule, there will be more incentive to connect tests to OVS patchwork. We get more automation as a side effect. However, we would still need to define who is the sub-maintainer or perhaps think on the "chain of trust"/"blindy committed" idea mentioned above. > * Some lack of review can be attributed to a reluctance to accept a > review from a reviewer who is at the same company as the patch > submitter. There is good reason for this, but it is certainly > possible to get quality reviews from a coworker, and perhaps we should > relax the convention. I also agree with that. I think OVS had that going on years ago and worked well. The same happens with OVN. It should be OK during devel phase when we can revert if something goes wrong. We may strict if it's close to release date though, if some are uncomfortable with this idea. > * A flip side of the above could be to codify the requirement for review > from a non-coworker. This would have the benefit of being able to > push back against requests to commit unreviewed long series on the > basis that it hasn't been reviewed by a third party. Good idea. That goes in the same direction of having rules to help maintainers to accept patches without require them to do an extensive review. Also, should we change 0-day bot to send reminders if a patch is sitting without a review for a certain period of time? Should we have rules like for example hardware offload - having ACK from another hardware offload company would be enough to get the patch accepted? Thanks, -- fbl _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
