Hi On Tue, May 12, 2020 at 5:07 PM Francesco Giudici <fgiudici@xxxxxxxxxx> wrote: > > > > On 12/05/20 11:24, Daniel P. Berrangé wrote: > > On Mon, May 11, 2020 at 12:16:24PM +0200, Marc-André Lureau wrote: > >> Hi, > >> > >> About "Move code to C++": > >> https://gitlab.freedesktop.org/spice/spice/-/merge_requests/62 > >> > >> I would like to know how the decision happened. As long as I have been > >> involved in the Spice project, we had open discussions and did > >> mandatory review for anything non-trivial. > >> > >> This change is substantial, and impacts the work and contribution of > >> others. Where did the discussion happen? Who reviewed the code > >> changes? > > > > Looking at that merge request, it is pretty surprising to see a 100 > > patch long series merged with no review comments visible on the code > > from other maintainers. > > > > Regards, > > Daniel > > > > I see your points: a proper discussion and fair review on the branch > would have been the way to go. > To have a fair overall picture btw, I think we should also consider some > other points: > $> git shortlog --since 01/01/2019 -s -n > 411 Frediano Ziglio > 29 Victor Toso > 20 Uri Lublin > 14 Francois Gouget > 11 Christophe Fergeau > 10 Snir Sheriber > 6 Eduardo Lima (Etrunko) > 6 Jonathon Jongsma > 6 Kevin Pouget > 6 Lukáš Hrázký > 5 James Le Cuirot > 3 Thiago Mendes > 2 Rosen Penev > 1 Benjamin Tissoires > 1 Christian Ehrhardt > 1 Douglas Paul > 1 Gilmar Santos Jr > 1 Jeremy White > 1 worldofpeace > 1 谢 昆明 To be "fair", the number of commits in spice server alone over 1.5y is not a very good metric. Certainly, Frediano's work is consequent, thank you, but it's also part of his job afaik. > > Frediano's branches don't get much reviews (if any... see the full list > of merged/closed MR in gitlab for the spice/spice project). I think we > all agree that his intention is good, which is to just move the project > forward. Wondering who would have looked into his 100 patches branch to > do a fair review in a reasonable time-frame. > I feel (at least partially) guilty for this situation. > > That said... at this point the branch has been merged. What are the > proposals now? > > Draft a more formal review/commit policy? What if a branch doesn't get a > fair review in an acceptable time-frame? Who will have the last word if > unanimity is not reached? What does an acceptable time-frame mean? If the work is important, the project maintainers should do a review in a timely manner. If nobody has enough time to review changes, isn't it because we, collectively, have more important things to do? The contributor who proposed such a change in the first place should realize that. > > Do you want to do a post-merge review to consider reverting the commits? > Do you want to have a detached "C" branch with the former code to be > kept as a "stable C" one? > Or what else? I am a past contributor to the Spice server, but I regularly have to read or debug the code. I keep co-maintaining the spice-gtk client, although it seems others are doing a pretty good job at helping me. Is there a problem with the Spice server maintenance? I don't know. What do you want me to say... I am disappointed by this change, the nature of the change, switching to c++, and the way it happened. It doesn't reflect practices I like in open-source projects and the way the Spice project has been developed in the past. Overall, the way it happened is detrimental to the project imho, and I would indeed revert the change if nobody reviewed it openly. The spice server should not to be taken so lightly, it doesn't deserve it either. _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel