Should PR be approved?

[monsieurtanuki]

I've just noticed several PRs (#3148 #3144 #3141 #3126 #3121 #3101) that have been merged in the last 10 days without any review or approval.

I understand that in some rare cases a review can be very problematic: when the contributor is the only one who understands a topic, who else is qualified to review?

But when we deal with "normal" flutter code, I don't agree. The code review is part of the process. It helps building a community of contributors. It helps building a common understanding of the programing language and of the app.

I would even say that PRs are supposed to fix issues, so the normal path would be first the "there's something we could improve" issue, then comments about this issue and setting a priority.
And only then there should be code (if decided so and with a high priority) and a PR. To be reviewed and approved.

My suggestions:

• create labels for PRs like "experts only" and "express review needed"
• tend to create PRs from issues
• tend to prioritize issues and to focus only on the high priorities (with the exception of "good first issue" for beginners, in order to enlist them)

@g123k @M123-dev @AshAman999 @VaiTon @teolemon What are your opinions?

[M123-dev]

Heyy @monsieurtanuki, for the PRs you mentioned, you can see that all of them were merged now 7 days ago, this was during the weekly meeting where we decided to merge the large refacoring PR and directly fixed problems which were caused by it. But still, I totally agree with your suggestions especially the labels sound great.

Also the priorization is keept managed here by @teolemon https://github.com/orgs/openfoodfacts/projects/7/views/1

[teolemon]

My bad that's on me (having merged), we should have taken the time to put everything in writing, and be less hasty.

[VaiTon]

Hey! I agree with the need of reviews for flutter code (app code). We could even set the "required reviews" number to 1 for me!

Despite that, I don't know about the issue and PR workflow, because a lot of time I find myself wanting to improve a thing (that could also not be related to flutter) and having to open an issue, then a PR if and only if the issue is considered important seems a bit excessive. I'm referring to PRs like #3180 or #3166.

About the times where the contributor is the only one that understands that part of the code / config files, having a required number of approvations does not lock the process because, as we do in openfoodfacts-server, we can "approve" the code by trusting the author on that particular subject.