Idea for PR reviewing

[Aaron Meurer]

One problem with PR reviewing is that it isn't always clear which PRs are ready to review, and which require more work. 

One solution that has been proposed in the past is these "PR: author's turn" and "PR: sympy's turn" labels. But the problem is that you can't change PR labels unless you have push access. So once a PR has a "PR: author's turn" label, the only way to remove it is if someone with push access removes it. And aside from that, people aren't diligent enough to always keep the labels updated.

But I just noticed that as I go through the PRs list, I generally skip those PRs that have failing status label (a red X), as that means that some tests have failed, so there is already some work to do on the author's part to fix the PR (I also generally skip PRs with WIP, unless I am specifically interested in them).

So my idea is to have some kind of "CI service" that lets any PR reviewer assign a status label to the PR, either a checkmark for "passes review" or an X for "needs work". The status itself could link to a comment that points out what needs to be done. That way, any PR that "needs work" will have a red X, and I can see when going through the list that I can skip it. 

The nice thing about this is that, because the status is only assigned to the most recent commit, as soon as the PR author pushes a new commit, the red X status for the "needs work" will go away. 

This could also bring accountability for our "all PRs must be reviewed" policy. 

Bitbucket actually has this feature built into their PR system, which is one thing I like about it, but I think it should be possible to do the same thing with GitHub with a bit of work. 

What do you think? Anyone have any idea how to implement something like this? Does anyone know if someone already has?

Aaron Meurer

[Isuru Fernando]

I think that's great idea, given the large number of open PRs.

Implementing this is very easy to do. What's hard is to find the correct workflow.

How do you trigger the red X status? A comment in the PR with a specific message?
How do you suggest turning the red X status off when the reviewer's comments are addressed, but no new commits are sent?


Isuru Fernando

--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at https://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6KVtuOF4CNBd2tUKr%2BOQSB0vSWbhkOLnAN19y7iZdW%3DPw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Aaron Meurer]

Yes, that's the question. 

You need some kind of webapp that runs somewhere to trigger the status. Perhaps as a UI you could reuse the existing label system, where setting a label triggers the app to set the status. Or maybe the webapp has a page that you can go to for each commit to trigger the status (the default status could be set as green, to give a link). And yes, you need some way to invert the status for a given commit, in case it's decided that no changes are in fact needed. 

Honestly, it would be better if GitHub implemented the sign-off UI directly. The most elegant (from a reviewer's point of view) solution I can think of is a browser extension that puts a simple "Sign off / Needs more work" button on each PR. But then reviewers have to install the extension...

Aaron Meurer

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CA%2B01voMmV5HYj-5KNFO2HwRGgaKjvq_b0ZN6QmcSkX3iaS7ZqQ%40mail.gmail.com.

[Isuru Fernando]

See https://github.com/isuruf/symengine/pull/19 for a demo.

Code is here, https://github.com/isuruf/sympy-pr-webapp

Isuru Fernando

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BFwknyQVq%3Dj2rgxvTQo8pEqqFGSwLh12svo5nKhNZgNQ%40mail.gmail.com.

[Aaron Meurer]

It seems several similar tools already exist. https://about.pullapprove.com/ and https://lgtm.co/ are two that were pointed out to me.  They both seem more focused on making sure a review happens before merging, which I'm less worried about. My main concern is seeing which PRs are ready for review.

Aaron Meurer

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CA%2B01voPhn8dOE-%3D%2B6%2BG0LwN2z3A5tZuGsuQ5pGPGgP2Lea93Mw%40mail.gmail.com.

[Ondřej Čertík]

One option is to close the PR if it's not ready, since I think the
author of the PR can reopen it. That solves the issue that the author
of the PR can't touch the labels.

>>>>> an email to sympy+un...@googlegroups.com.

>>>>> To post to this group, send email to sy...@googlegroups.com.
>>>>> Visit this group at https://groups.google.com/group/sympy.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/sympy/CAKgW%3D6KVtuOF4CNBd2tUKr%2BOQSB0vSWbhkOLnAN19y7iZdW%3DPw%40mail.gmail.com.
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "sympy" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send

>>>> an email to sympy+un...@googlegroups.com.

>>>> To post to this group, send email to sy...@googlegroups.com.
>>>> Visit this group at https://groups.google.com/group/sympy.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/sympy/CA%2B01voMmV5HYj-5KNFO2HwRGgaKjvq_b0ZN6QmcSkX3iaS7ZqQ%40mail.gmail.com.
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "sympy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to sympy+un...@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BFwknyQVq%3Dj2rgxvTQo8pEqqFGSwLh12svo5nKhNZgNQ%40mail.gmail.com.
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+un...@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CA%2B01voPhn8dOE-%3D%2B6%2BG0LwN2z3A5tZuGsuQ5pGPGgP2Lea93Mw%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/d/optout.
>
>

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+un...@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6LVac3rkYiVVDTdtk7RyESQqa2cA2fsLK7FKsd7oVzrgQ%40mail.gmail.com.

[Aaron Meurer]

I'm not a fan of that. Closing gives people the impression that the pull request is being rejected.

Aaron Meurer

>>>>> an email to sympy+unsubscribe@googlegroups.com.

>>>>> To post to this group, send email to sy...@googlegroups.com.
>>>>> Visit this group at https://groups.google.com/group/sympy.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/sympy/CAKgW%3D6KVtuOF4CNBd2tUKr%2BOQSB0vSWbhkOLnAN19y7iZdW%3DPw%40mail.gmail.com.
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "sympy" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send

>>>> an email to sympy+unsubscribe@googlegroups.com.

>>>> To post to this group, send email to sy...@googlegroups.com.
>>>> Visit this group at https://groups.google.com/group/sympy.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/sympy/CA%2B01voMmV5HYj-5KNFO2HwRGgaKjvq_b0ZN6QmcSkX3iaS7ZqQ%40mail.gmail.com.
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "sympy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to sympy+unsubscribe@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BFwknyQVq%3Dj2rgxvTQo8pEqqFGSwLh12svo5nKhNZgNQ%40mail.gmail.com.
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+unsubscribe@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CA%2B01voPhn8dOE-%3D%2B6%2BG0LwN2z3A5tZuGsuQ5pGPGgP2Lea93Mw%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+unsubscribe@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6LVac3rkYiVVDTdtk7RyESQqa2cA2fsLK7FKsd7oVzrgQ%40mail.gmail.com.

>
> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "sympy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.

To post to this group, send email to sy...@googlegroups.com.
Visit this group at https://groups.google.com/group/sympy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVArxrT1F%3DH4EZ%2B754mF8cxrAOOxMwU5gJ6w971hn0Qi%2Bw%40mail.gmail.com.

[Ondřej Čertík]

On Thu, Sep 8, 2016 at 12:09 PM, Aaron Meurer <asme...@gmail.com> wrote:
> I'm not a fan of that. Closing gives people the impression that the pull
> request is being rejected.

I know. GitHub should improve this workflow a lot.

Ondrej

[Jason Moore]

I agree that closing is not a good idea.

Jason
moorepants.info
+01 530-601-9791

--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at https://groups.google.com/group/sympy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.

[Aaron Meurer]

So it looks like GitHub has added direct support for this. The UI so far is a little confusing. Hopefully they will improve that. 

Aaron Meurer

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.

[Ondřej Čertík]

I was just about to post here:

https://github.com/blog/2256-a-whole-new-github-universe-announcing-new-tools-forums-and-features

Will this make it obvious when a PR is waiting for the author to do
more work, and can the author "flip a switch" and we can quickly see
that the work got done?

Ondrej

On Wed, Sep 14, 2016 at 12:19 PM, Aaron Meurer <asme...@gmail.com> wrote:
> So it looks like GitHub has added direct support for this. The UI so far is
> a little confusing. Hopefully they will improve that.
>
> Aaron Meurer
>
> On Thu, Sep 8, 2016 at 3:59 PM, Jason Moore <moore...@gmail.com> wrote:
>>
>> I agree that closing is not a good idea.
>>
>>
>> Jason
>> moorepants.info
>> +01 530-601-9791
>>
>> On Thu, Sep 8, 2016 at 12:15 PM, Ondřej Čertík <ondrej...@gmail.com>
>> wrote:
>>>
>>> On Thu, Sep 8, 2016 at 12:09 PM, Aaron Meurer <asme...@gmail.com> wrote:
>>> > I'm not a fan of that. Closing gives people the impression that the
>>> > pull
>>> > request is being rejected.
>>>
>>> I know. GitHub should improve this workflow a lot.
>>>
>>> Ondrej
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "sympy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to sympy+un...@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+un...@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/d/optout.
>
>

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+un...@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6L%2BQ8Js%2BAANRSL7ebqKtAujzdYyq03eQr76JeBqdwYDew%40mail.gmail.com.

[Aaron Meurer]

I can't tell. Like I said, the UI is pretty confusing. We'll have to play around with it.

Aaron Meurer

>>> email to sympy+unsubscribe@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+unsubscribe@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+unsubscribe@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6L%2BQ8Js%2BAANRSL7ebqKtAujzdYyq03eQr76JeBqdwYDew%40mail.gmail.com.

>
> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "sympy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.

To post to this group, send email to sy...@googlegroups.com.
Visit this group at https://groups.google.com/group/sympy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVA_tKqLrRfxHNgGQ6rVSQqGi4pA%3DVHJNNcUcdO25rpY9A%40mail.gmail.com.

[Ondřej Čertík]

I went there:

https://github.com/sympy/sympy/settings/branches/master

and checked "Require pull request reviews before merging" (including
administrators), then reviewed & approved this PR:


https://github.com/sympy/sympy/pull/11601

But it says "Required statuses must pass before merging". But all
statues pass! Then I unchecked "Require pull request reviews before
merging", and now it can be merged... I think it's a bug at GitHub...


Ondrej

>> >>> email to sympy+un...@googlegroups.com.

>> >>> To post to this group, send email to sy...@googlegroups.com.
>> >>> Visit this group at https://groups.google.com/group/sympy.
>> >>> To view this discussion on the web visit
>> >>>
>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.
>> >>> For more options, visit https://groups.google.com/d/optout.
>> >>
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> >> Groups
>> >> "sympy" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> >> an

>> >> email to sympy+un...@googlegroups.com.

>> >> To post to this group, send email to sy...@googlegroups.com.
>> >> Visit this group at https://groups.google.com/group/sympy.
>> >> To view this discussion on the web visit
>> >>
>> >> https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.
>> >>
>> >> For more options, visit https://groups.google.com/d/optout.
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "sympy" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an

>> > email to sympy+un...@googlegroups.com.

>> > To post to this group, send email to sy...@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/sympy.
>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/d/msgid/sympy/CAKgW%3D6L%2BQ8Js%2BAANRSL7ebqKtAujzdYyq03eQr76JeBqdwYDew%40mail.gmail.com.
>> >
>> > For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+un...@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CADDwiVA_tKqLrRfxHNgGQ6rVSQqGi4pA%3DVHJNNcUcdO25rpY9A%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+un...@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6JghYv4nA3fN8WPqRM5Qd4PjVxD3EK-DKLL2DXx_3KdYw%40mail.gmail.com.

[Ondřej Čertík]

On Thu, Sep 15, 2016 at 6:30 PM, Ondřej Čertík <ondrej...@gmail.com> wrote:
> I went there:
>
> https://github.com/sympy/sympy/settings/branches/master
>
> and checked "Require pull request reviews before merging" (including
> administrators), then reviewed & approved this PR:
>
>
> https://github.com/sympy/sympy/pull/11601
>
> But it says "Required statuses must pass before merging". But all
> statues pass! Then I unchecked "Require pull request reviews before
> merging", and now it can be merged... I think it's a bug at GitHub...

So I reported it to GitHub Support.

Ondrej

[Aaron Meurer]

Maybe it means someone other than the merger has to review it. 

Aaron Meurer 

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVCVx8cAPWDk70swbYf%3DDWkbx%3D9%3DSYUf%3Dx0pdDmXyxeHHA%40mail.gmail.com.

[Ondřej Čertík]

GitHub got back to me, they said it's a bug, that they have an issue
open for it internally. So after they fix it, I think this will do
exactly what we need --- we'll enable the "require approval" check
box, and then it will fail the status if the PR is reviewed and
require more work (i.e. not approved). Then we can filter PRs by the
status to see PRs that are either not reviewed, or the author pushed
more commits after the review.

One problem that I can still see is that sometimes a discussion is
resolved by commenting, that a further commit is not needed. I don't
know how that fits into this workflow.

But I think this is the way to go overall, it should be an improvement
and we'll have to figure out the details.

Ondrej

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BXuQnVWt-y0g-dgmXt%3DQhziWS%3DX8r5uKnrWF2OOuC1Kg%40mail.gmail.com.

[Ondřej Čertík]

GitHub reached out to me again, that they fixed it. Indeed, it seems
to be fixed, so I have enabled the "require review", even for
administrators. Now if you want to merge a PR, just click on "review"
and click "approve". Then it will be allowed to merge.

[Ondřej Čertík]

On Wed, Sep 21, 2016 at 8:33 PM, Ondřej Čertík <ondrej...@gmail.com> wrote:
> GitHub reached out to me again, that they fixed it. Indeed, it seems
> to be fixed, so I have enabled the "require review", even for
> administrators. Now if you want to merge a PR, just click on "review"
> and click "approve". Then it will be allowed to merge.

So if you click here:

https://github.com/sympy/sympy/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aopen%20status%3Asuccess

it shows only PRs, that have "status: success". Unfortunately it shows
PRs even if they are not approved yet. So we still have to figure out
a way to filter by the review status. I am reporting it to GitHub now.

[Aaron Meurer]

So now it's impossible to merge your own PR (someone else has to review it first). Hopefully this doesn't become an issue. If you need me to review your PR, feel free to ping me on Gitter.

Aaron Meurer

>>> >>> >>> email to sympy+unsubscribe@googlegroups.com.

>>> >>> >>> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >>> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >>> To view this discussion on the web visit
>>> >>> >>>
>>> >>> >>>
>>> >>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.
>>> >>> >>> For more options, visit https://groups.google.com/d/optout.
>>> >>> >>
>>> >>> >>
>>> >>> >> --
>>> >>> >> You received this message because you are subscribed to the Google
>>> >>> >> Groups
>>> >>> >> "sympy" group.
>>> >>> >> To unsubscribe from this group and stop receiving emails from it,
>>> >>> >> send
>>> >>> >> an

>>> >>> >> email to sympy+unsubscribe@googlegroups.com.

>>> >>> >> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >> To view this discussion on the web visit
>>> >>> >>
>>> >>> >>
>>> >>> >> https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.
>>> >>> >>
>>> >>> >> For more options, visit https://groups.google.com/d/optout.
>>> >>> >
>>> >>> >
>>> >>> > --
>>> >>> > You received this message because you are subscribed to the Google
>>> >>> > Groups
>>> >>> > "sympy" group.
>>> >>> > To unsubscribe from this group and stop receiving emails from it,
>>> >>> > send
>>> >>> > an

>>> >>> > email to sympy+unsubscribe@googlegroups.com.

>>> >>> > To post to this group, send email to sy...@googlegroups.com.
>>> >>> > Visit this group at https://groups.google.com/group/sympy.
>>> >>> > To view this discussion on the web visit
>>> >>> >
>>> >>> >
>>> >>> > https://groups.google.com/d/msgid/sympy/CAKgW%3D6L%2BQ8Js%2BAANRSL7ebqKtAujzdYyq03eQr76JeBqdwYDew%40mail.gmail.com.
>>> >>> >
>>> >>> > For more options, visit https://groups.google.com/d/optout.
>>> >>>
>>> >>> --
>>> >>> You received this message because you are subscribed to the Google
>>> >>> Groups
>>> >>> "sympy" group.
>>> >>> To unsubscribe from this group and stop receiving emails from it, send
>>> >>> an

>>> >>> email to sympy+unsubscribe@googlegroups.com.

>>> >>> To post to this group, send email to sy...@googlegroups.com.
>>> >>> Visit this group at https://groups.google.com/group/sympy.
>>> >>> To view this discussion on the web visit
>>> >>>
>>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVA_tKqLrRfxHNgGQ6rVSQqGi4pA%3DVHJNNcUcdO25rpY9A%40mail.gmail.com.
>>> >>> For more options, visit https://groups.google.com/d/optout.
>>> >>
>>> >>
>>> >> --
>>> >> You received this message because you are subscribed to the Google
>>> >> Groups
>>> >> "sympy" group.
>>> >> To unsubscribe from this group and stop receiving emails from it, send
>>> >> an

>>> >> email to sympy+unsubscribe@googlegroups.com.

>>> >> To post to this group, send email to sy...@googlegroups.com.
>>> >> Visit this group at https://groups.google.com/group/sympy.
>>> >> To view this discussion on the web visit
>>> >>
>>> >> https://groups.google.com/d/msgid/sympy/CAKgW%3D6JghYv4nA3fN8WPqRM5Qd4PjVxD3EK-DKLL2DXx_3KdYw%40mail.gmail.com.
>>> >>
>>> >> For more options, visit https://groups.google.com/d/optout.
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "sympy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to sympy+unsubscribe@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CADDwiVCVx8cAPWDk70swbYf%3DDWkbx%3D9%3DSYUf%3Dx0pdDmXyxeHHA%40mail.gmail.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "sympy" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to sympy+unsubscribe@googlegroups.com.

>> To post to this group, send email to sy...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/sympy.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BXuQnVWt-y0g-dgmXt%3DQhziWS%3DX8r5uKnrWF2OOuC1Kg%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "sympy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.

To post to this group, send email to sy...@googlegroups.com.
Visit this group at https://groups.google.com/group/sympy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVD8jWxuoOUcjWg2Qfjk%3DQC8w-27RgyBLgQDNHEKxHFfeg%40mail.gmail.com.

[Aaron Meurer]

So here's a problem. At https://github.com/sympy/sympy/pull/11648, I requested changes (an X review), and the author pushed some fixes. But it still lists me with an X, as requesting changes, and says the PR can't be merged. I'm curious if others are able to merge that PR, or if I have to revert my review first.

Aaron Meurer

[Ondřej Čertík]

I can't merge either, until you change your review.

>>> >>> >>> >>> email to sympy+un...@googlegroups.com.

>>> >>> >>> >>> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >>> >>> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >>> >>> To view this discussion on the web visit
>>> >>> >>> >>>
>>> >>> >>> >>>
>>> >>> >>> >>>
>>> >>> >>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVBecdOpF%3D3qqgooaiZwepf_%2Baur7kiC-Ufu25p_6upyHQ%40mail.gmail.com.
>>> >>> >>> >>> For more options, visit https://groups.google.com/d/optout.
>>> >>> >>> >>
>>> >>> >>> >>
>>> >>> >>> >> --
>>> >>> >>> >> You received this message because you are subscribed to the
>>> >>> >>> >> Google
>>> >>> >>> >> Groups
>>> >>> >>> >> "sympy" group.
>>> >>> >>> >> To unsubscribe from this group and stop receiving emails from
>>> >>> >>> >> it,
>>> >>> >>> >> send
>>> >>> >>> >> an

>>> >>> >>> >> email to sympy+un...@googlegroups.com.

>>> >>> >>> >> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >>> >> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >>> >> To view this discussion on the web visit
>>> >>> >>> >>
>>> >>> >>> >>
>>> >>> >>> >>
>>> >>> >>> >> https://groups.google.com/d/msgid/sympy/CAP7f1Agjqo08YJiLcu19-xmW9okttHOn9rm2gth-rF1Y772XJA%40mail.gmail.com.
>>> >>> >>> >>
>>> >>> >>> >> For more options, visit https://groups.google.com/d/optout.
>>> >>> >>> >
>>> >>> >>> >
>>> >>> >>> > --
>>> >>> >>> > You received this message because you are subscribed to the
>>> >>> >>> > Google
>>> >>> >>> > Groups
>>> >>> >>> > "sympy" group.
>>> >>> >>> > To unsubscribe from this group and stop receiving emails from
>>> >>> >>> > it,
>>> >>> >>> > send
>>> >>> >>> > an

>>> >>> >>> > email to sympy+un...@googlegroups.com.

>>> >>> >>> > To post to this group, send email to sy...@googlegroups.com.
>>> >>> >>> > Visit this group at https://groups.google.com/group/sympy.
>>> >>> >>> > To view this discussion on the web visit
>>> >>> >>> >
>>> >>> >>> >
>>> >>> >>> >
>>> >>> >>> > https://groups.google.com/d/msgid/sympy/CAKgW%3D6L%2BQ8Js%2BAANRSL7ebqKtAujzdYyq03eQr76JeBqdwYDew%40mail.gmail.com.
>>> >>> >>> >
>>> >>> >>> > For more options, visit https://groups.google.com/d/optout.
>>> >>> >>>
>>> >>> >>> --
>>> >>> >>> You received this message because you are subscribed to the
>>> >>> >>> Google
>>> >>> >>> Groups
>>> >>> >>> "sympy" group.
>>> >>> >>> To unsubscribe from this group and stop receiving emails from it,
>>> >>> >>> send
>>> >>> >>> an

>>> >>> >>> email to sympy+un...@googlegroups.com.

>>> >>> >>> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >>> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >>> To view this discussion on the web visit
>>> >>> >>>
>>> >>> >>>
>>> >>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVA_tKqLrRfxHNgGQ6rVSQqGi4pA%3DVHJNNcUcdO25rpY9A%40mail.gmail.com.
>>> >>> >>> For more options, visit https://groups.google.com/d/optout.
>>> >>> >>
>>> >>> >>
>>> >>> >> --
>>> >>> >> You received this message because you are subscribed to the Google
>>> >>> >> Groups
>>> >>> >> "sympy" group.
>>> >>> >> To unsubscribe from this group and stop receiving emails from it,
>>> >>> >> send
>>> >>> >> an

>>> >>> >> email to sympy+un...@googlegroups.com.

>>> >>> >> To post to this group, send email to sy...@googlegroups.com.
>>> >>> >> Visit this group at https://groups.google.com/group/sympy.
>>> >>> >> To view this discussion on the web visit
>>> >>> >>
>>> >>> >>
>>> >>> >> https://groups.google.com/d/msgid/sympy/CAKgW%3D6JghYv4nA3fN8WPqRM5Qd4PjVxD3EK-DKLL2DXx_3KdYw%40mail.gmail.com.
>>> >>> >>
>>> >>> >> For more options, visit https://groups.google.com/d/optout.
>>> >>>
>>> >>> --
>>> >>> You received this message because you are subscribed to the Google
>>> >>> Groups
>>> >>> "sympy" group.
>>> >>> To unsubscribe from this group and stop receiving emails from it,
>>> >>> send an

>>> >>> email to sympy+un...@googlegroups.com.

>>> >>> To post to this group, send email to sy...@googlegroups.com.
>>> >>> Visit this group at https://groups.google.com/group/sympy.
>>> >>> To view this discussion on the web visit
>>> >>>
>>> >>> https://groups.google.com/d/msgid/sympy/CADDwiVCVx8cAPWDk70swbYf%3DDWkbx%3D9%3DSYUf%3Dx0pdDmXyxeHHA%40mail.gmail.com.
>>> >>> For more options, visit https://groups.google.com/d/optout.
>>> >>
>>> >> --
>>> >> You received this message because you are subscribed to the Google
>>> >> Groups
>>> >> "sympy" group.
>>> >> To unsubscribe from this group and stop receiving emails from it, send
>>> >> an

>>> >> email to sympy+un...@googlegroups.com.

>>> >> To post to this group, send email to sy...@googlegroups.com.
>>> >> Visit this group at https://groups.google.com/group/sympy.
>>> >> To view this discussion on the web visit
>>> >>
>>> >> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BXuQnVWt-y0g-dgmXt%3DQhziWS%3DX8r5uKnrWF2OOuC1Kg%40mail.gmail.com.
>>> >>
>>> >> For more options, visit https://groups.google.com/d/optout.
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "sympy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an

>>> email to sympy+un...@googlegroups.com.

>>> To post to this group, send email to sy...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/sympy.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/sympy/CADDwiVD8jWxuoOUcjWg2Qfjk%3DQC8w-27RgyBLgQDNHEKxHFfeg%40mail.gmail.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sympy+un...@googlegroups.com.

> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at https://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BZMOAsxGAE%3DoWH14RrzLZeN2v0Xc0xraNxsUDDDb3ZgQ%40mail.gmail.com.

[Aaron Meurer]

More updates on this: you can now dismiss pull request reviews (ignore
them for the purposes of required reviews).
https://github.com/blog/2265-dismissing-reviews-on-pull-requests. So
it might be worth trying reenabling required reviews again, as
outdated negative reviews can now be dismissed without requiring the
original reviewer to do so. The only thing it actually forces now is
that someone, other than the PR author, has to review the PR, meaning
self-merges without someone else reviewing are disallowed.

By the way, if anyone is interested, I wrote a (pretty ranty) blog
post about the GitHub reviews feature
https://asmeurer.github.io/blog/posts/github-reviews-gripes/.

Aaron Meurer

> To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVA98USdMdLqTxorseUcoRa1V6vvyhOoXf0rigN8otk-PQ%40mail.gmail.com.