[Spice-devel] About decisions and reviews

[Francesco Giudici]

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