Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paramétrage des reviews de Pull Request #868

Open
mabhub opened this issue Dec 12, 2019 · 2 comments
Open

Paramétrage des reviews de Pull Request #868

mabhub opened this issue Dec 12, 2019 · 2 comments
Assignees

Comments

@mabhub
Copy link
Contributor

mabhub commented Dec 12, 2019

On est quelques-uns à utiliser la mécanique des code reviews accessible dans les Pull Request.
Pour ajouter un peu de cohérence à ceci, serait-il possible d'activer dans les paramètres l'option qui permet d'invalider les reviews déjà faites lorsque de nouveaux commits (ou push force) sont apportée à une PR ?

image

Il faut pour cela activer la protection de la branche, puis indiquer la nécessité de N review pour pouvoir valider la PR. Il devient alors possible de faire sauter les reviews lorsque la PR est modifiée. Même dans ce cas, les administrateurs du projet ne sont, par défaut, pas contraints à cette nécessité de review.

Cette mécanique permettrait que, lorsque quelqu'un approuve une PR, cette approbation ne persiste pas alors que l'auteur a potentiellement changé le contenu de la PR. Le relecteur a alors à nouveau la possibilité de renouveler son approbation ou bien de suggérer des changements.

@dunglas
Copy link
Member

dunglas commented Dec 14, 2019

Est-ce que ces options ont vraiment un intérêt dans notre cas ? L'invalidation de la review à chaque commit est, d’expérience sur d'autres projets, très pénible : la correction d'une typo, un changement de CS ou d'un style CSS va entraîner la nécessité d'une nouvelle action humaine.
Pareil pour les branches protégées, ça a peu d'intérêt pour ce projet, et ça va rendre plus pénible l'intégration des signatures et les petites modifications/corrections que l'on fait au script Ruby.

@mabhub
Copy link
Contributor Author

mabhub commented Dec 14, 2019

Ce n'est pas indispensable, c'est juste un peu plus cohérent avec la mécanique de review. Ça évite qu'une PR semble validée par X personne alors que son contenu a pu complètement changer depuis la validation.
Personnellement, je ne vois que peu d'intérêt au fait de valider une PR, si ce statut persiste malgré les changements ultérieurs.

Par contre le fait de valider ou non est indépendant du fait de commenter le contenu de la PR. L'un ne conditionne pas l'autre. Quant à la protection, elle n'est que très peu contraignante pour les mainteneurs du projet. Elle permet essentiellement d'éviter le vandalisme (parfois involontaire).

Comme d'hab, je ne me vexerai pas que ce ne soit pas appliqué ! 😉
Par contre il est probable que je ne prenne plus la peine de "approve" les différentes PR (non pas que j'estime que mes reviews soient nécessaires, je me disais juste que ça aidait).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants