Re: [sage-devel] Re: VOTE: Revert merged PR with unreviewed dependencies

[Kwankyu Lee]

-1 to be in sync with my vote in #37796.

On Friday, April 19, 2024 at 5:35:57 AM UTC+9 Marc Culler wrote:

>
>
> On Thursday, April 18, 2024 at 12:47:36 PM UTC-5 David Roe wrote:
>
> On Thu, Apr 18, 2024 at 1:43 PM Matthias Koeppe wrote:
>
> I will first note that the title of this post is misleading.
> Everything that was merged has been reviewed -- as noted, many months ago.
>
>
> I agree that everything was reviewed. 
>
>
> And that is why I vote -1 on reverting the merge.
>
> - Marc
>
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/4fc94cac-c462-4a96-ae9f-be7be57c9fe8n%40googlegroups.com.

Re: [sage-devel] Re: VOTE: Revert merged PR with unreviewed dependencies

[Marc Culler]

On Thursday, April 18, 2024 at 12:47:36 PM UTC-5 David Roe wrote:

On Thu, Apr 18, 2024 at 1:43 PM Matthias Koeppe wrote:

I will first note that the title of this post is misleading.
Everything that was merged has been reviewed -- as noted, many months ago.


I agree that everything was reviewed. 


And that is why I vote -1 on reverting the merge.

- Marc

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/b14df2d2-bfea-4cbb-afd4-706ad925e739n%40googlegroups.com.

Re: [sage-devel] Re: VOTE: Revert merged PR with unreviewed dependencies

[David Roe]

On Thu, Apr 18, 2024 at 1:51 PM Matthias Koeppe 
wrote:

> David, none of this explains the misleading use of the word "unreviewed".


I believe that it does.  If there was confusion, hopefully this exchange
can help clarify it for others.
David


> On Thursday, April 18, 2024 at 10:47:36 AM UTC-7 David Roe wrote:
>
>> On Thu, Apr 18, 2024 at 1:43 PM Matthias Koeppe 
>> wrote:
>>
>>> I will first note that the title of this post is misleading.
>>> Everything that was merged has been reviewed -- as noted, many months
>>> ago.
>>>
>>
>> I agree that everything was reviewed.  However review refers not only to
>> the action of giving approval (which was done), but also to the status of
>> the PR as indicated by Positive Review, Needs Review, and Needs Work
>> labels.  This is the standard used by the release management scripts, as
>> well as our community understanding of what it means for a PR to be ready
>> for merging.  Under this definition, #36951 and #36676 did not have
>> positive review at the time that #36964 was merged.
>> David
>>
>> On Thursday, April 18, 2024 at 8:54:26 AM UTC-7 David Roe wrote:
>>>
 Hi all,
 Sage has had a review process for over 15 years, but a combination of
 recent changes has led to the merging of a PR into sage-10.4.beta3 of a
 change (#36964 ) that I
 believe should not (yet) have been merged.  In #37796
  I created a PR to revert
 the change, which was opposed by the author of the original change.  After 
 some
 voting
 
 using the disputed PR policy
 ,
 Matthias has asked
 
 for a vote on sage-devel about this reversion, in accordance with the
 section that "This process is intended as a lower-intensity method for
 resolving disagreements, and full votes on sage-devel override the process
 described below."  I am therefore asking you to vote (+1 means merge
 #37796  in order to
 revert #36964 ).

 First, here are the relevant parts of the history of this particular
 change:

 - #36964  was created on
 December 25 by Matthias, positively reviewed
 
 by Kwankyu on Decemebr 27, disputed, received enough votes
 
 to get a positive review on April 7, and was merged
 
 by Volker on April 12.  It had dependencies: #37667,
 #36951
 , and #36676
 .  While #37667
  had positive review and
 was already been merged, the other two were still disputed: they had
 received an initial positive review but others objected and discussion was
 ongoing.

 - #37667  is not disputed.

 - #36951  was created on
 December 23 by Matthias, positively reviewed
 
 by Kwankyu on January 1, disputed, received enough votes
 
 (3-1) to change to positive review on April 7, had a clarification to bring
 back to (3-2) and remove positive review, then was included in the merge of
 #36964 . On April 13,
 John Palmieri voted in favor
 ,
 so the current vote stands at 4-2, enough for the 2-1 threshold in order to
 get positive review under the disputed voting process.

 - #36676  was created on
 November 8 by Matthias, positively reviewed
 
 by John Palmieri on November 15, and then disputed.  The most recent count
 was 6-4 in favor
 
 (falling short of the 2-1 ratio needed under the disputed voting process);
 since then I voted
 
 in favor, it was included in the merge of #36964
 , and then Martin voted
 

Re: [sage-devel] Re: VOTE: Revert merged PR with unreviewed dependencies

[Matthias Koeppe]

David, none of this explains the misleading use of the word "unreviewed".


On Thursday, April 18, 2024 at 10:47:36 AM UTC-7 David Roe wrote:

> On Thu, Apr 18, 2024 at 1:43 PM Matthias Koeppe  
> wrote:
>
>> I will first note that the title of this post is misleading.
>> Everything that was merged has been reviewed -- as noted, many months ago.
>>
>
> I agree that everything was reviewed.  However review refers not only to 
> the action of giving approval (which was done), but also to the status of 
> the PR as indicated by Positive Review, Needs Review, and Needs Work 
> labels.  This is the standard used by the release management scripts, as 
> well as our community understanding of what it means for a PR to be ready 
> for merging.  Under this definition, #36951 and #36676 did not have 
> positive review at the time that #36964 was merged.
> David
>
> On Thursday, April 18, 2024 at 8:54:26 AM UTC-7 David Roe wrote:
>>
>>> Hi all,
>>> Sage has had a review process for over 15 years, but a combination of 
>>> recent changes has led to the merging of a PR into sage-10.4.beta3 of a 
>>> change (#36964 ) that I 
>>> believe should not (yet) have been merged.  In #37796 
>>>  I created a PR to revert 
>>> the change, which was opposed by the author of the original change.  After 
>>> some 
>>> voting 
>>>  
>>> using the disputed PR policy 
>>> , 
>>> Matthias has asked 
>>>  
>>> for a vote on sage-devel about this reversion, in accordance with the 
>>> section that "This process is intended as a lower-intensity method for 
>>> resolving disagreements, and full votes on sage-devel override the process 
>>> described below."  I am therefore asking you to vote (+1 means merge 
>>> #37796  in order to revert 
>>> #36964 ).
>>>
>>> First, here are the relevant parts of the history of this particular 
>>> change:
>>>
>>> - #36964  was created on 
>>> December 25 by Matthias, positively reviewed 
>>>  
>>> by Kwankyu on Decemebr 27, disputed, received enough votes 
>>>  
>>> to get a positive review on April 7, and was merged 
>>>  
>>> by Volker on April 12.  It had dependencies: #37667, 
>>> #36951 
>>> , and #36676 
>>> .  While #37667 
>>>  had positive review and 
>>> was already been merged, the other two were still disputed: they had 
>>> received an initial positive review but others objected and discussion was 
>>> ongoing.
>>>
>>> - #37667  is not disputed.
>>>
>>> - #36951  was created on 
>>> December 23 by Matthias, positively reviewed 
>>>  
>>> by Kwankyu on January 1, disputed, received enough votes 
>>>  
>>> (3-1) to change to positive review on April 7, had a clarification to bring 
>>> back to (3-2) and remove positive review, then was included in the merge of 
>>> #36964 . On April 13, John 
>>> Palmieri voted in favor 
>>> , 
>>> so the current vote stands at 4-2, enough for the 2-1 threshold in order to 
>>> get positive review under the disputed voting process.
>>>
>>> - #36676  was created on 
>>> November 8 by Matthias, positively reviewed 
>>>  
>>> by John Palmieri on November 15, and then disputed.  The most recent count 
>>> was 6-4 in favor 
>>>  
>>> (falling short of the 2-1 ratio needed under the disputed voting process); 
>>> since then I voted 
>>>  
>>> in favor, it was included in the merge of #36964 
>>> , and then Martin voted 
>>> against.
>>>
>>>
>>> At issue is the PR #36676 , 
>>> where discussion was still ongoing when #36964 
>>>  was 

Re: [sage-devel] Re: VOTE: Revert merged PR with unreviewed dependencies

[David Roe]

On Thu, Apr 18, 2024 at 1:43 PM Matthias Koeppe 
wrote:

> I will first note that the title of this post is misleading.
> Everything that was merged has been reviewed -- as noted, many months ago.
>

I agree that everything was reviewed.  However review refers not only to
the action of giving approval (which was done), but also to the status of
the PR as indicated by Positive Review, Needs Review, and Needs Work
labels.  This is the standard used by the release management scripts, as
well as our community understanding of what it means for a PR to be ready
for merging.  Under this definition, #36951 and #36676 did not have
positive review at the time that #36964 was merged.
David

On Thursday, April 18, 2024 at 8:54:26 AM UTC-7 David Roe wrote:
>
>> Hi all,
>> Sage has had a review process for over 15 years, but a combination of
>> recent changes has led to the merging of a PR into sage-10.4.beta3 of a
>> change (#36964 ) that I
>> believe should not (yet) have been merged.  In #37796
>>  I created a PR to revert
>> the change, which was opposed by the author of the original change.  After 
>> some
>> voting
>> 
>> using the disputed PR policy
>> ,
>> Matthias has asked
>> 
>> for a vote on sage-devel about this reversion, in accordance with the
>> section that "This process is intended as a lower-intensity method for
>> resolving disagreements, and full votes on sage-devel override the process
>> described below."  I am therefore asking you to vote (+1 means merge
>> #37796  in order to revert
>> #36964 ).
>>
>> First, here are the relevant parts of the history of this particular
>> change:
>>
>> - #36964  was created on
>> December 25 by Matthias, positively reviewed
>> 
>> by Kwankyu on Decemebr 27, disputed, received enough votes
>>  to
>> get a positive review on April 7, and was merged
>>  by
>> Volker on April 12.  It had dependencies: #37667,
>> #36951
>> , and #36676
>> .  While #37667
>>  had positive review and
>> was already been merged, the other two were still disputed: they had
>> received an initial positive review but others objected and discussion was
>> ongoing.
>>
>> - #37667  is not disputed.
>>
>> - #36951  was created on
>> December 23 by Matthias, positively reviewed
>> 
>> by Kwankyu on January 1, disputed, received enough votes
>> 
>> (3-1) to change to positive review on April 7, had a clarification to bring
>> back to (3-2) and remove positive review, then was included in the merge of
>> #36964 . On April 13, John
>> Palmieri voted in favor
>> ,
>> so the current vote stands at 4-2, enough for the 2-1 threshold in order to
>> get positive review under the disputed voting process.
>>
>> - #36676  was created on
>> November 8 by Matthias, positively reviewed
>>  by
>> John Palmieri on November 15, and then disputed.  The most recent count was 
>> 6-4
>> in favor
>> 
>> (falling short of the 2-1 ratio needed under the disputed voting process);
>> since then I voted
>>  in
>> favor, it was included in the merge of #36964
>> , and then Martin voted
>> against.
>>
>>
>> At issue is the PR #36676 ,
>> where discussion was still ongoing when #36964
>>  was merged.  The reversion
>> of this PR proposed is purely for process reasons (I voted in favor of
>> #36676  before all this
>> happened!).  The 5 Sage developers opposed to #36676
>>  deserve to have our
>> processes