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

2024-04-26 Thread seb....@gmail.com
Sure! But the fact was that reviewers objected to a commit in PR A that was approved in PR B, so their objection was simply ignored! Please don't understand me wrong. I'm not saying that it was your responsibility. It is still the responsibility of the reviewers to make sure that this does not

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

2024-04-25 Thread Volker Braun
On Tuesday, April 23, 2024 at 6:24:51 PM UTC+2 seb@gmail.com wrote: The problem with this is: if there are commits on a branch that are reviewed in more than one PR, the question is: does *positive review* mean *all* or *some* PR's? The review is for a single PR, not for individual

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

2024-04-23 Thread seb....@gmail.com
> *Just go with how github works, which is positive review = ready to merge and "files changed" shows the actual changes that this PR implements.* The problem with this is: if there are commits on a branch that are reviewed in more than one PR, the question is: does *positive review* mean

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

2024-04-23 Thread David Roe
The vote was 8-4. I'm setting the PR back to positive review. David On Sat, Apr 20, 2024 at 5:04 AM Volker Braun wrote: > It was merged because it was positively reviewed. > > Neither I nor the merge script reads every ticket description and looks > through the text whether any dependency is

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

2024-04-20 Thread Volker Braun
It was merged because it was positively reviewed. Neither I nor the merge script reads every ticket description and looks through the text whether any dependency is mentioned that has not yet been reviewed. We can try to build such a Rube Goldberg machine, but I would very much argue against

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

2024-04-19 Thread Travis Scrimshaw
+1 for merging #37796. Volker, I would appreciate if you could say something about how #36964 was merged. It would be useful to understand the process with merging this, rather than guessing the intent. Additionally, I thought we didn't merge things when the dependencies have not been merged

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

2024-04-18 Thread G. M.-S.
-1 If something has been done that should be undone, I very much trust Volker to take care of it when he can, without the need for endless time-consuming discussions and votes. Best, Guillermo On Thu, 18 Apr 2024 at 17:54, David Roe wrote: > Hi all, > Sage has had a review process for over

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

2024-04-18 Thread julian...@fsfe.org
+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+unsubscr...@googlegroups.com. To view this discussion on the web visit

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

2024-04-18 Thread Michael Orlitzky
On Thu, 2024-04-18 at 11:54 -0400, David Roe wrote: > I am therefore asking you to vote (+1 means merge #37796 > in order to revert #36964 > ). +1 -- You received this message because you are subscribed

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

2024-04-18 Thread David Joyner
On Thu, Apr 18, 2024 at 11:54 AM 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 >

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

2024-04-18 Thread Dima Pasechnik
+1 to reverting the wrong merge On 18 April 2024 16:54:08 BST, 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 )