VOTE: disputed PRs

[David Roe]

With no further discussion on this thread, I'm calling a vote on a new process for resolving disagreements on a PR.

Proposal

It is now allowed to vote on disputed PRs directly on Github rather than bringing them to sage-devel.  Working things out amicably is preferable, and anyone is welcome to ask on sage-devel for more eyes on a PR.  If you notice a serious issue with a PR, it is acceptable to change it to Needs Work (and make a comment!) as an initial step, but if the author or reviewer do not agree then process below should be followed instead. This process is intended as a lower-intensity method for resolving disagreements, and full votes on sage-devel override the process described below.

a. When there is disagreement about whether a PR should be merged, anyone may mark a PR as disputed.

b. There is no scheduled vote, but rather an ongoing poll based on opinions expressed by developers on the PR (these opinions can be expressed via previous positive reviews or explicit comments giving approval).  The PR author is presumed to vote in favor; if they give up or no longer favor the PR they have the right to close the PR overall without any further voting.

c. If the total number of positive votes is at least twice the number of negative votes, anyone involved may set the status to positive review; if the total number of positive votes is less than twice the number of negative votes, anyone involved may set the status to needs review.  When either of these actions is taken, the person changing the status must list the people they are counting as positive and negative votes in a comment using @ mentions.

d. The final decision on merging a disputed PR remains with the release manager, and we encourage the release manager to give enough time for everyone to express an opinion.

Voting will be open until Wednesday, March 13.

David

[Kwankyu Lee]

+1

[Dima Pasechnik]

David, 

how about team members who are blocked on GitHub.

For GitHub voting to work, this has to be sorted out first.

Dima

--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAChs6_n4az3_s16E%3DANOv_o%2B0SvavHwnpqKWYuOznGWTJoXqEg%40mail.gmail.com.

[Matthias Koeppe]

+1

... although I expect that further clarifications will be needed once we have tried to apply this process.

[Edgar Costa]

+1

[G. M.-S.]

+1

Guillermo

[Vincent Delecroix]

+1

> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CANnG189YucyTQqiSYM7%2BBbgDHSauYdzsP9G%3DDP5gXYhgp15KTQ%40mail.gmail.com.

[John H Palmieri]

Dima, I think that if anyone is incapable of posting to a particular PR, they should send email to someone who can post and ask them to record the person's vote, resulting in a comment like "I am posting to record 1 negative vote from X, 2 positive votes from Y and Z".

[John H Palmieri]

+1

On Monday, March 4, 2024 at 12:23:39 AM UTC-8 David Roe wrote:

[kcrisman]

Dima, I think that if anyone is incapable of posting to a particular PR, they should send email to someone who can post and ask them to record the person's vote, resulting in a comment like "I am posting to record 1 negative vote from X, 2 positive votes from Y and Z".

Yes, that would also go for those who do not choose to use GH.   

[Dima Pasechnik]

I think this doesn't work. E.g. the proposal talks about setting PRs to "needs work", but banned by the PR's author team members can't do this.

That's why, as I said already, bans break the normal workflow, be it reviewing or voting.

[Nathan Dunfield]

+1

[Travis Scrimshaw]

Essentially +1

but might want to consider cases when its 2 vs 1 as requiring at least one other person involved. (Sorry for being late to realize this.)

Best,
Travis

[David Lowry-Duda]

+1

- DLD

On 03:23 Mon 04 Mar 2024, David Roe wrote:
>With no further discussion on this thread

><https://groups.google.com/g/sage-devel/c/XDvKkMRoDk4>, I'm calling a vote

>on a new process for resolving disagreements on a PR.
>

>*Proposal*

>It is now allowed to vote on disputed PRs directly on Github rather than
>bringing them to sage-devel. Working things out amicably is preferable,
>and anyone is welcome to ask on sage-devel for more eyes on a PR. If you
>notice a serious issue with a PR, it is acceptable to change it to Needs
>Work (and make a comment!) as an initial step, but if the author or
>reviewer do not agree then process below should be followed instead. This
>process is intended as a lower-intensity method for resolving
>disagreements, and full votes on sage-devel override the process described
>below.
>a. When there is disagreement about whether a PR should be merged, anyone
>may mark a PR as disputed.
>b. There is no scheduled vote, but rather an ongoing poll based on opinions
>expressed by developers on the PR (these opinions can be expressed via
>previous positive reviews or explicit comments giving approval). The PR
>author is presumed to vote in favor; if they give up or no longer favor the
>PR they have the right to close the PR overall without any further voting.
>c. If the total number of positive votes is at least twice the number of

>negative votes, anyone involved may set the status to *positive review*; if

>the total number of positive votes is less than twice the number of

>negative votes, anyone involved may set the status to *needs review*. When

>either of these actions is taken, the person changing the status must list
>the people they are counting as positive and negative votes in a comment
>using @ mentions.
>d. The final decision on merging a disputed PR remains with the release
>manager, and we encourage the release manager to give enough time for
>everyone to express an opinion.
>
>Voting will be open until Wednesday, March 13.
>David
>

>--
>You received this message because you are subscribed to the Google Groups "sage-devel" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
>To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAChs6_n4az3_s16E%3DANOv_o%2B0SvavHwnpqKWYuOznGWTJoXqEg%40mail.gmail.com.

--
David Lowry-Duda <da...@lowryduda.com> <davidlowryduda.com>

[seb....@gmail.com]

+1

[kcrisman]

Having just encountered this "in action", I have a suggestion:

"There is no scheduled vote, but rather an ongoing poll based on opinions expressed by developers on the PR (these opinions can be expressed via previous positive reviews or explicit comments giving approval).  The PR author is presumed to vote in favor; if they give up or no longer favor the PR they have the right to close the PR overall without any further voting."

It would be helpful for anyone explicitly voting +/-1 to either link to a previous comment or make a new actual comment (beyond the vote) to clarify.  This is particularly if there have been new commits since the initial dispute, because for an outside reviewer it can be hard to untangle all the various comments before the disputed tag is placed on the PR.  

I don't know if this means I'm voting for or against the proposal, though!  Friendly amendment, perhaps.

[kcrisman]

It would be helpful for anyone explicitly voting +/-1 to either link to a previous comment or make a new actual comment (beyond the vote) to clarify.  This is particularly if there have been new commits since the initial dispute, because for an outside reviewer it can be hard to untangle all the various comments before the disputed tag is placed on the PR.  

(Which would apply to any emailed requests as well.) 

[David Roe]

The vote has passed.  There are currently 36 open disputed PRs.

Given the extensive comments on some of these PRs, I would agree that we should follow some version of Karl-Dieter's suggestion.  In particular, while the author of the PR remains automatically in favor, I'll ask that people who gave positive reviews affirm their continued support for the PR by commenting (or forwarding their vote to someone who can comment).  With such a large set of disputed tickets, while we give people a chance to make their opinions known please wait until the end of the day Friday (say, US/Pacific time) before changing status to positive review or needs review as described in the policy.  Volker will hopefully not merge any of these disputed tickets for a little while while we work through the new policy.

If there are snags in the process, we can work through them here on sage-devel.

Also note that I do not anticipate this new process to be a magic bullet that resolves all of our disagreements that underlie these disputes.  We will be announcing the results of the vote for new members of the Sage Code of Conduct committee soon, and I hope that the new committee can discuss ways to continue to resolve the disagreements.  In the meantime, please be respectful of each other, both here and on github.

I am not going to have time to encode this policy into our Developer's Guide for a couple weeks; anyone else is welcome to do so and I'd be happy to offer a review.

David

On Mon, Mar 11, 2024 at 1:44 PM kcrisman <kcri...@gmail.com> wrote:

It would be helpful for anyone explicitly voting +/-1 to either link to a previous comment or make a new actual comment (beyond the vote) to clarify.  This is particularly if there have been new commits since the initial dispute, because for an outside reviewer it can be hard to untangle all the various comments before the disputed tag is placed on the PR.  

(Which would apply to any emailed requests as well.) 

--

You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/bb229d60-d204-4d04-bc3d-2c88515a389dn%40googlegroups.com.

[Matthias Koeppe]

David,

Would you clarify if there is any difference in process between the PRs currently set to "positive review" and those currently set to "needs review"?

[Kwankyu Lee]

... if there is any difference in process between the PRs currently set to "positive review" and those currently set to "needs review"?

In my opinion, all disputed PRs waiting for voting should be reset to "needs review" status. 

Kwankyu

[David Roe]

Sorry for the delay in responding; I have had much time today.

The code of conduct committee's intention in changing the status on some of the disputed tickets was to note ways in which participants had not followed our previous standards for setting review status.  At this point, given the policy that just passed, I personally don't see any practical difference between the tickets currently being set to positive review and set to needs review, since either status can be updated to the other based on the current voting tally.

Voting ends on the new membership for the committee shortly (please send in your votes now if you've been waiting).  I'd like to ask the new committee if they have an opinion on how to handle the situation; personally I'm fine with Kwankyu's suggestion of changing all positive review tickets to needs review as a way to have changes made by others be in a positive direction.

David

--

You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/c3532c5d-acdf-4dda-a0ac-385719c0f1c4n%40googlegroups.com.