Skip to content

Latest commit

 

History

History
101 lines (71 loc) · 5.8 KB

2-contributions.md

File metadata and controls

101 lines (71 loc) · 5.8 KB

Comment Contribuer

Signaler un problème

Si vous rencontrez un problème, contactez-nous par email.

Soumettre une modification

Les pull requests sont bienvenues ! N’hésitez pas à nous en parler à l’avance. La démarche est habituelle : faites un fork, créez une branche, faites un PR. Pour les petites corrections de fautes d’orthographe, n’hésitez pas à proposer une modification directement depuis github.com.

Style de code

Au delà du style de syntaxe, nous essayons de suivre quelques principes. RDVS-S a déjà plusieurs années, et a été développé par plusieurs équipes successives. C’est pour cette raison qu’on peut trouver plusieurs façons de faire au sein du projet. Aujourd’hui, nous essayons d’aller dans cette direction:

  1. Écrire moins de code, et du code plus simple
  • Éviter de créer trop de classes auxiliaires, comme les presenters, form objects, services, etc.
  • Éviter les concerns/modules qui ne servent qu’à un seul modèle ; réserver ça aux fonctionnalités communes.
  • Préférer plutôt la proximité du code avec l’endroit où il est appelé.
  1. Plutôt du rails monolithique
  • Minimiser le JS utilisant des API spécifiques
  • à terme, on utilisera hotwire
  1. Moins de dépendances externes
  • à terme, on retirera webpack. Peut-être lors la migration à Rails 7.
  1. Documenter le code
  • Concrètement, dans les modèles, regrouper en tête de fichier, sous forme de sections:
    • attributs,
    • relations,
    • validations et hooks
    • scopes
  1. ActiveRecord
  • utiliser les relation through autant que possible pour construire les queries
  1. Pour les tests, utiliser les helpers et rspec avec parcimonie
  • Par exemple, les let, subject, etc, doivent rester proches de leur lieu d’utilisation, quitte à être répétés dans un autre context.
  1. Pour manipuler des dates et heures, il est recommandé d'utiliser ActiveSupport::TimeWithZone plutôt que des Time ou des DateTime. Plus d'explications dans cette PR.
  2. Lorsque l'on veut fusionner des requêtes SQL structurellement différentes, il est recommandé d'utiliser de sous-requêtes plutôt que de passer de récupérer puis réutiliser des listes d'IDs. La méthode de helper .where_id_in_subqueries peut être utilisée pour construire facilement des sous-requêtes.

Linters

Dans l’ensemble, nous suivons les conventions de Ruby on Rails Rails best practice et The rails Style Guide. Nous utilisons aussi largement rubocop; dans tous les cas, le linter alertera en cas de problèmes.

  • Faire tourner tous les linters :
make lint                 Run all linters
make lint_rubocop         Ruby linter
make lint_slim            Slim Linter
make lint_brakeman        Security Checker
  • Demander à Rubocop de corriger les problèmes qu’il rencontre :
make autocorrect          Fix autocorrectable lint issues

Tests

Note : nos bonnes pratiques sur les tests sont à lire ici : Bonnes pratiques de test

Nous utilisons RSpec pour écrire nos tests. En principe, la base de données de tests est créée automatiquement.

Les feature tests utilisent Capybara et ont besoin de Chrome (et de chromedriver) pour s’exécuter. Pour plusieurs devs sous MacOS dans l'équipe, il a fallu ajouter cette ligne à son fichier /etc/hosts pour pouvoir faire tourner les tests en local :

127.0.0.1 www.rdv-solidarites-test.localhost
  • Lancer tous les tests
make test                 Run all tests
make test_unit            Run unit tests in parallel
make test_features        Run feature tests
  • Lancer tous les tests d’un fichier
bin/rspec file_path/file_name_spec.rb
  • Lancer un test en particulier
bin/rspec file_path/file_name_spec.rb:line_number

Workflow de merge des pull requests

Afin de garder un historique git lisible et navigable par git blame, nous recommandons l'une de ces deux façons de merger une PR :

  • Utiliser "Squash and merge" si les commits de la PR n'apportent pas individuellement de valeur explicative sur le contexte.
  • Utiliser "Create a merge commit" si la PR contient des commits qui permettent de mieux comprendre les différents changements indépendants introduits dans la PR.

Au sein de notre projet, il est assumé que la majorité du contexte autour du changement est trouvable dans la PR et non dans les commits. Cependant, il est tout à fait possible de conserver ses commits si on les a bien créés pour qu'ils permettent d'obtenir rapidement une synthèse du contexte via git blame.

Par exemple, si au sein d'une même PR on effectue un (petit 🤞) refactor puis une évolution fonctionnelle, il est apprécié que le refactor fasse l'objet d'un commit séparé.

Note : il est possible de réécrire son historique de commits juste avant de merger, si des commits correctifs ont été ajoutés durant la revue.

Un point d'attention : si vous avez mergé la branche production dans votre branche de feature pendant la vie de votre PR, veillez à ce que ces commits de merge ne finissent pas dans production. Pour ce faire :

  • si vous utilisez un squash merge, ces commits vont disparaître
  • si vous mergez dans production le plus pratique est de rebase votre branche sur production avant de merger.

Note : lorsque votre feature branch n'est plus à jour par rapport à production, GitHub affiche un avertissement "This branch is out-of-date with the base branch" et vous propose de remédier à la situation. Ce faisant, on déclenche une CI qui teste le code tel qu'il serait s'il était mergé. Si cette CI passe, on peut alors merger.