Github: "awaiting core review" even if changes requested


(Jeroen Demeyer) #1

Whenever I leave a review on a pull request on Github, the status tag always goes from awaiting review to awaiting core review regardless of whether the review is approving or requesting changes. Wouldn’t awaiting changes be more appropriate in the latter case? This is probably specific to reviews by non-core developers.


#2

Yeah that’s the behavior whenever a non core dev leaves review (whether it was approving/requesting changes).


(Stéphane Wirtel) #3

Hi @jdemeyer

Already discussed on the ML: core-workflow. this status is boring because you want to do the reviews before the core-devs but it’s the default workflow.

https://mail.python.org/archives/list/core-workflow@python.org/thread/OIWMZC5R3JHSKO4VLCUTJWY43FC3552V/


(Terry Jan Reedy) #4

Core-dev reviews are treated differently because: 1. Other things being equal, we are more confident of their quality. 2. Merging requires that at least one core dev review the PR or an existing review. Core devs can ignore the status tag distinction and judge reviews themselves. I do.


(Jeroen Demeyer) #5

To me, this is sending again the message that you don’t really take reviews of non-core developers seriously. Despite what Guido also said in Howto engage Python contributors in the long term? I still have the feeling that the core developers don’t care about reviews from non-core developers. And the replies here are reinforcing that feeling.


(Antoine Pitrou) #6

@jdemeyer Personally I would take into account reviews from non-core developers based on what I know of their experience and competence on the topic at hand. For example, I know that you wrote a difficult PEP on improving the CPython C API, so I would certainly find you competent enough to review PRs dealing with C API intricacies.

Sometimes I actually request advice from non-core developers, if I know that they are proficient on a topic or that they may give feedback as advanced users.

That said, I’m not doing many reviews these days either, so perhaps that won’t change your day-to-day experience significantly, sorry…


(Brett Cannon) #7

Please realize we have had people come in and incorrectly nitpick on PEP 8 styling and leave a Changes Requested review for that. And so it’s a balancing act of either we let anyone come in and cause a PR to be flagged as needing a change or we do what the current practice is and let non-core reviews act as help/guidance for a core dev to come along and hopefully have an easier time reviewing the PR since someone else came along and already did an initial pass.