Re: About decisions and reviews

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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  谢 昆明

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?

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?

best,
Francesco

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]