[Spice-devel] About decisions and reviews

[Marc-André Lureau]

Hi

On Tue, May 12, 2020 at 5:25 PM Frediano Ziglio <fziglio at redhat.com> 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?
> >
> > thanks
> >
>
> Hi,
>   your description of the project looks a bit unrealistic to me.
>
> About 6 months ago we moved to Gitlab for code reviews. The change was
> announced to the ML which was the old way of doing. The procedure with
> Gitlab is more or less:
> - you prepare and post a MR to Gitlab (previously you would send a mail);
> - people wanting to review should be subscribed to Gitlab, receive a
>   notification and they can decide to open discussions, comment or propose
>   changes (previously they had to reply to mails);
> - code is changed based on discussions, comments and proposals;
> - there is some final acknowledge of the MR, MR is merged.
>
> Now:
> - MR was opened in the public, questions and comments were on the cover
>   letter;
> - people had the time to comments and discussions. Only one public
>   discussion raised, correctly addressed and marked as solved, no
>   other comments following for a month so MR was correctly reviewed;
> - there was some final acknowledge so it was merged.

"MR was correctly reviewed", without a single comment on code over ~100 patches?

>
> The question here is "was it enough"? Honestly this reminded my the
> "epsilon" constant in Math. Something really close to zero... but still
> positive! I raised the problem of the poor reviews multiple time, trying
> to push people. I was discouraged multiple times. Correctly contributions
> are made voluntarily and it's not good to force people to work voluntarily.
> In a democracy people that does not vote are basically not counted.
>
> What is your expectation? Every hunk addressed? What is the frame time?
> Maybe finish reading and commenting the cover letter for the end of
> the year?

It's not a question of time. If something needs to be addressed, we
put the resources behind.

Does switching to c++ fits that category? Probably not (certainly not
if you ask me).

>
> Some time ago, when reviews were still done with ML, but the issue was clearly
> present I tried to stop reviewing myself. The result was clear. Most of the
> external code contributions were simply ignored, no replies at all.
> Some reached almost 2 months without any reply... then I started replying
> again!
> The situation has been like this for years and it get worse and worse.
> Today Victor linked a 3 years old proposal for some mouse enhancement.

Let's not point fingers, but there are probably many reasons for that.
It's not a reason to take a many year project, decide what it's good
or not in private, and push unreviewed commits.

>
> The definition of contributor involve contributions. Keeping apart non-code
> contributions what are the code contributions? Surely new code, reviews,
> comments and proposal on code. Now, if people does not review, open or
> continue discussion or proposal are they real contributors? How many
> stable contributors can Spice project state?

You are going to scare even more contributors, by deciding by yourself
or in private and not involving them.

> Let's look at the major feature in the last Spice server release:
> Windows support and WebSockets. Beside being myself strong contributing to
> both (first was just a rainy weekend challenge) the second had a bit too
> long history.
> The patches for WebSockets were sent years ago reviewed quickly without much
> attempts to help the writer solving the raised issues. Some proposal were
> pretty unrealistic (like "why don't you rewrite everything?"). This was not
> even the first attempt to add this feature to Spice. The total realizations
> took 7 years. I would saying not a great way to involve new contributors.
> I want the project to evolve more quicker.

There are many contributions, in many projects, that can take years to
be merged. And many contributions never get merged.

Spice is no exception. If there are enough interest, there are enough
people behind it to polish the work. If not, then you cannot put the
project at risk by rushing work.


-- 
Marc-André Lureau