Github: "awaiting core review" even if changes requested

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.

1 Like

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

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/

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.

3 Likes

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.

1 Like

@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…

2 Likes

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.

2 Likes

A potential solution to improving upon the balancing act might be to have an intermediate role of reviewer which is a step above a contributor, but not a core dev. This would be similar in nature to the already existing triager group, but specific to PR reviews. It would also provide the ability for the reviewer to modify or add labels, such as skip news and skip issue for any minor PRs that do not warrant creating an issue for, such as documentation typos. Ultimately, the goal of this role would be to reserve the core developer’s time as much as possible. The final approval for merging the PR would still be on the core developers, but it would allow them to spend less time on doing so if reviewers could assist with some of the other parts.

Based on my experience as a newer contributor (started within the last month), so far I have felt that my code reviews have been quite well received for the most part. Admittedly there have been a few times where questions were ignored, but usually I do feel that my feedback is valued.

The only issue is that there’s no intermediate goal for people such as myself to work towards. Someone could spend a considerable amount of time reviewing code and have the same exact recognition as someone who was only made a single contribution. It may also serve well as another stepping stone towards eventually becoming a core developer. In general, I think the python community could significantly benefit from having more specialized roles beyond contributor, triager, and core developer.

Would it be appropriate to start a new discussion topic to discuss the above topic at length and perhaps have a vote from the community? This is related to the issue being discussed, but could easily be a topic on its own.

Edit: I just realized after receiving clarification that the role I was describing would apply to a triager: Proposal: Create "Bug Triage" team on GitHub - #27 by aeros

I can assure you that this is not the case!

The general problem is that recognition on GitHub is superficially based on labels and up-votes and green squares, but those are entirely meaningless.

Actual recognition is if people interact with you (even if they disagree). Core developers will remember your reviews, they always have before all these bells and whistles existed.

3 Likes

Thank you, that’s good to know. I think I may have gotten a little bit caught up in the superficial titles and the icons/labels next to the names. In general, the icons and titles do provide a material representation of sorts, and provide a tiny bit of extra motivation for the person doing it, but you’re completely right that they’re ultimately meaningless.