Les revues de code

Une revue de code ou code review consiste à vérifier les ajouts et modifications afin d’améliorer la qualité générale du code. L’objectif est de faire des retours ou commentaires en rapport avec :

la fonctionnalité implémentée : est-ce qu’elle respecte le cahier des charges ?
des oublis : un élément à rajouter ou du code inutile à supprimer
des choix techniques : conseils sur des alternatives, complexité du code, etc.
des guidelines propres au projet

Pour les modifications purement visuelles comme les espaces, le nommage des variables ou le fameux, il est recommandé d’avoir des commentaires ou corrections automatiques avec des outils comme phpcs ou php-cs-fixer.

Quand faire une revue de code ?

La revue de code se fait la plupart du temps à la fin du développement d’une fonctionnalité ou d’un correctif mais peut se faire dès que le code est mis en commun.

Les plateformes de versionning les plus connues intègrent directement des possibilités de revue de code. On retrouve cette fonction dans Github, Gitlab, Bitbucket, Azure DevOps, Phabricator. Dans le cas de systèmes ne le permettant pas, il existe des outils spécialisés comme gerrit qui s’intègrent directement à git.

Pour plus d’informations sur le sujet, Google a mis en ligne un ensemble de documents concernant les pratiques de leurs ingénieurs.

L’intérêt d’une checklist

En fonction de votre équipe, vous allez vite remarquer que certaines erreurs vont souvent revenir. Il peut être intéressant dans ce cas de créer une checklist que chaque reviewer pourra utiliser pour s’assurer de ne rien oublier.

Pour démarrer, je vous mets ci-dessous la checklist proposée par la New York University dans le cadre de son cours Effective Code Review et Software Carpentry, des projets pour apprendre les bonnes pratiques de programmation.

Général

Est-ce que le code fonctionne? Est-ce qu’il effectue son fonctionnement attendu, la logique est correcte, etc.
Est-ce que tout le code se comprend facilement?
Est-ce qu’il correspond à vos conventions de code? Cela comprend généralement la position des accolades, les noms de fonction et de variable, la taille des lignes, l’indentation, le format et les commentaires.
Est-ce qu’il y a du code redondant ou dupliqué?
Le code est-il aussi modulaire que possible?
Une variable globale peut-elle être remplacée?
Y-a-t-il des lignes de codes commentées?
Les boucles ont-elles une taille définie et une condition de fin correct?
Est-ce que les noms utilisés sont parlant et informent du sens de la variable ou fonction?

Performance

Y-a-t-il des optimisations évidentes pour améliorer la performance?
Est-ce qu’une partie du code peut être remplacé par une librairie ou une fonction du langage?
Reste-t-il du code concernant les logs ou le debug à supprimer?

Sécurité

Est-ce que toutes les données d’entrées sont vérifiées et encodées? Au niveau de leur type, taille, format et portée
Quand des utilitaires tiers sont utilisées, les retours d’erreurs sont-ils contrôlés?
Les valeurs de sortie sont-elles vérifiées et encodées?
Les valeurs incorrectes sont-elles gérées?

Documentation

Y-a-t-il des commentaires et décrivent-ils les objectifs du code?
Toutes les fonctions sont-elles commentées?
Y-a-t-il des comportements inhabituels ou des cas limites décrits?
Est-ce que l’usage et le fonctionnement des librairies tierces sont documentés?
Est-ce que les structures de données et les unités de mesures sont expliquées?
Y-a-t-il du code incomplet ? Si oui, doit-il être supprimé ou marqué avec un TODO ou un autre moyen?

Tests

Le code est-il testable ? Le code doit être structuré pour qu’il n’ajoute pas trop ou ne cache pas de dépendance, qu’il soit capable d’initialiser ses objets. Des frameworks de tests peuvent être utilisés, etc.
Des tests existent et sont-ils compréhensibles?
Est-ce que les tests unitaires vérifient réellement que le code effectue la fonctionnalité attendue?
Est-ce que du code de test peut être remplacé par une API existante?