On Fri, May 15, 2020 at 07:05:50PM +0200, Francesco Giudici wrote: > Hi, > the community around the SPICE project always tried to follow one > fundamental, implicit rule for accepting code contributions to the project: > every merge request (beside trivial patches) should be reviewed and acked at > least by one before getting merged. > While everyone agrees with this fundamental rule, the actual status of some > SPICE projects makes the rule impractical to let the project move forward. > Let's consider the spice/spice project as an example: the number of > contributions is very low, both on the commit side (only 4 different > contributors with more than 1 commit from the beginning of the year, and a > single contributor with 90% of commits) and on the review side (in the last > 40 merge requests before the C++ switch one, 21 had no comments). > The x11spice project is another example: we have only 4 contributors from > the beginning of the year (and a single contributor holding 70% of the > commits) and the reviews on the gitlab merge requests have been provided by > two people only, each one reviewing the merge requests of the other. I think these paragraphs here are the highlighting the key issue. There just is not sufficient contribution to SPICE in general. I think the most important thing is to consider why the SPICE project has such a low rate of contribution, and more importantly what steps can be done to make SPICE more interesting to attract contributors. For a project that has such a large codebase and featureset, it is not healthy to have such a small community. The "bus factor" is waay too low here, such that SPICE would be in serious trouble as a project if one or two main contributors moved onto to different work. So for any changes in process, it is wise to consider what effects those change have on people SPICE would like to attract as new contributors. For example, if new feature work by an existing contributor is discussed out of band between two contributors, instead of via merge request comments this sets up new contributors up as second class citizenships - the out of band discussions are essentially invisible to them, so they can't see or understand the rationale for decisions. This makes it harder for them to learn how to effectively contribute to the project. This is the big thing that concerned me about the merge request that adopted C++. Reading between the lines I get the impression this was indeed discussed between the several contributors but out of band, instead of on the merge request which had no comments. Thus that discussion is invisible to any 3rd party contributor interested in SPICE. I think this risks discouraging people from contributed to SPICE, so it is something to be wary of doing too often, especially for really big technical changes. > For the sake of having the projects being able to move forward with a > reduced number of contributors/reviewers, the proposal is to *allow* a > maintainer to merge a Merge Request without an explicit ack if the three > following conditions are met: > 1) The Merge Request has been pending for at least 3 weeks without getting > new comments > 2) The Merge Request submitter has kept asking a review on a weekly basis Note since spice is using gitlab, it is possible to explicitly tag people by name in the comments to ask for reviews, and/or assign people as a reviewer. I'd suggest to make use of such tagging, so that people can be aware they they are being explicitly asked for feedback if something has been sitting for a long time. > 3) There are no pending nacks on the Merge Request > > Note that having patches reviewed would still be the preferred way. If at > any time the number of contributors would raise again, we can switch back to > the mandatory review rule. Until then the priority is to allow the project > to move forward. Contributors are unlikely to magically rise again on their own, without proactive work to make SPICE a more attractive project to contribute to. This is a very difficult problem for projects in general, not just SPICE, to which I don't have any magic answers. At least try to think about what you like to see from projects when you are approaching them as a new contributor, and then make sure SPICE follows these practices where ever possible / practical. Code being pushed with no review is a turn-off to me as a external contributor, unless the project is clearly setup as a single-author personal project, rather than a team project. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel