Re: [Spice-devel] About decisions and reviews
Hi On Thu, May 14, 2020 at 2:32 PM Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > Hi > > On Mon, May 11, 2020 at 12:16 PM Marc-André Lureau < > marcandre.lur...@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? >> >> > Since no real discussion nor public review took place, I propose to revert > the change and add a HACKING file to make the rules clear. See: > > https://gitlab.freedesktop.org/spice/spice/-/merge_requests/91 > > Based on the discussion there, and the fact that a majority of developper are willing to take the change, I closed the MR. Fwiw, beside the fact that the change wasn't done with review nor discussion (which on its own is detrimental), I think using C++ is wrong: - C++ is much more complex than C, and fewer developper know it. A good C++ developper is an even better C developper. - in our common ecosystem, QEMU, Linux, the GNOME desktop, are 90% C projects, and perhaps? 1% C++? (I don't know a single C++ system/core project tbh - only apps like KDE/LibreOffice/Firefox) - C++ projects are switching away from it (ex Firefox), I don't know of a core/system project talking about switching to C++ these days. - switching to C++ is going to be costly, as it is endless, and it will leave a bitter taste of previous/existing C. We could have used that time for other things... I would be pleasantly surprised if such refactoring would lead to regressions. - C has better interoperability story than C++, especially because the code was GObjectified. We could have added/exported more friendly API/bindings for GNOME easily. Overall, it's a waste of effort to me. What is the overall direction of the project? Except a few language improvements for a lower number of developpers, what does C++ bring us that we couldn't do with C for our users? Nevertheless, I will try to invest some time reviewing the changes. I hope we can agree on better project rules and directions for the future. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
Hi On Thu, May 14, 2020 at 2:32 PM Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > Hi > > On Mon, May 11, 2020 at 12:16 PM Marc-André Lureau < > marcandre.lur...@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? >> >> > Since no real discussion nor public review took place, I propose to revert > the change and add a HACKING file to make the rules clear. See: > > https://gitlab.freedesktop.org/spice/spice/-/merge_requests/91 > I propose a spice-space page derived from libvirt "Project governance": https://gitlab.freedesktop.org/spice/spice-space-pages/-/merge_requests/18 (readable version https://gitlab.freedesktop.org/spice/spice-space-pages/-/blob/9a1907de182f9f64450a7348e9e6a1423dfa580a/governance.rst ) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
Hi On Mon, May 11, 2020 at 12:16 PM Marc-André Lureau < marcandre.lur...@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? > > Since no real discussion nor public review took place, I propose to revert the change and add a HACKING file to make the rules clear. See: https://gitlab.freedesktop.org/spice/spice/-/merge_requests/91 Frediano, I suggest you open a thread about switching the code to c++. thanks -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
Hi On Tue, May 12, 2020 at 10:08 PM Francesco Giudici wrote: > > > > On 12/05/20 19:13, Marc-André Lureau wrote: > > Hi > > > > On Tue, May 12, 2020 at 5:07 PM Francesco Giudici > > 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. > My point here is that in the last 1.5y there was very little > contribution to the project apart from Frediano. Same on the review > side. In this context, I can understand why the branch was merged. I > have my opinion, but here I don't want to say he did it right or he did > it wrong: I'm just saying I can understand why he did. I don't understand the justification, honestly. The spice server is not overwhelmed with MR, and there is no rush to switch to C++. > > > > >> > >> 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. > > Well, agreeing on what is "important" is not easy, as it is personal. > Any contribution is important to the submitter ;-) It's a collective work, not somebody's project with its personal priority or wishes. > But my point is, let's make the rules clearer, so to try to avoid > similar situations in the future. We have had rules for years. Anything non-trivial has mandatory reviews. And in practice, we try to go through review all the time. > > > >> > >> 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
Re: [Spice-devel] About decisions and reviews
Hi, On Tue, May 12, 2020 at 10:08:46PM +0200, Francesco Giudici wrote: > So, let's move forward... Frediano merged it, this is done. > What would you propose to do now? Let's move the discussion on > what to do now :-) I agree that this is the best course of action. My suggestion was to ping people that used to be involved in Spice and are proficient in C++ and possibly would enjoy some wine and/or chocolates as good incentive to make time to review/test it (before the merge). So, options that come to mind now: 1) Revert, wait review, reviewed, (merging again or not) 2) Revert, wait review, never reviewed, (cyber dust or a fork) 3) Revert and fork it. I have a quite heavy trust in Frediano so my expectation of a fork is that it would become quite hard to GObject server to stay in parity with spice-cpp without new contributors. Cheers, Victor signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
On 12/05/20 19:13, Marc-André Lureau wrote: Hi On Tue, May 12, 2020 at 5:07 PM Francesco Giudici 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. My point here is that in the last 1.5y there was very little contribution to the project apart from Frediano. Same on the review side. In this context, I can understand why the branch was merged. I have my opinion, but here I don't want to say he did it right or he did it wrong: I'm just saying I can understand why he did. 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. Well, agreeing on what is "important" is not easy, as it is personal. Any contribution is important to the submitter ;-) But my point is, let's make the rules clearer, so to try to avoid similar situations in the future. 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. I like the love you have for the project. Really, great to see it. It is also ok to disagree and argue. But when you say "taken so lightly, it doesn't deserve it" you are putting blame there... Hey, we have all the same goal to make SPICE better and better. Frediano merged the branch to improve the project, not to make it worse... and he proved to love the project too. We are all on the same side! So, let's move forward... Frediano merged it, this is done. What would you propose to do now? Let's move the discussion on what to do now :-) Francesco ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
Hi On Tue, May 12, 2020 at 5:07 PM Francesco Giudici 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
Hi On Tue, May 12, 2020 at 5:25 PM Frediano Ziglio 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 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
> > 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. 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? 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. 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? 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. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] About decisions and reviews
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 -- |: 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] About decisions and reviews
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 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
