Que regardez-vous quand vous relisez la pull request React de quelqu'un ?

La justesse du flux de données d'abord : où vit l'état, si les effets sont nécessaires et ont des dépendances honnêtes, et si les clés sont stables. Ensuite les bords visibles par l'utilisateur, c'est-à-dire les états de chargement, vides et d'erreur, plus l'accès clavier et lecteur d'écran. Ensuite la performance, seulement là où elle est mesurable. Le style et le nommage viennent en dernier, et tout ce qu'un linter ou un formateur peut trancher ne devrait pas être un commentaire de revue.

Je lis la description et les tests d'abord, puis le diff, parce que je veux savoir ce que le changement est censé faire avant de juger comment il le fait. Ma première passe porte sur le flux de données. Où vit cet état, pourrait-il vivre plus bas, cet effet est-il vraiment nécessaire ou est-ce de l'état dérivé, les dépendances sont-elles honnêtes ou quelqu'un a-t-il discrètement fait taire la règle de lint, et les clés de liste sont-elles stables. La deuxième passe porte sur les bords, là où la plupart des bugs partent en production : que rend ceci pendant le chargement, quand la liste est vide, et quand la requête échoue, et puis-je m'en servir au clavier. La troisième est la performance, mais seulement si je peux pointer un coût réel, pas une impression. J'essaie de marquer mes commentaires comme bloquants ou comme simple remarque, parce que les chipotages non étiquetés bloquent des pull requests pendant des jours. Et je ne commente jamais le formatage ou l'ordre des imports ; si ça compte, ça appartient au linter, pas à mon avis.