FIX #6557 CONTRIBUTING Describe our dev best practices
This commit is contained in:
parent
88df8a888e
commit
e56b3e4a7a
2 changed files with 113 additions and 3 deletions
103
doc/Contributions/Pratiques-de-dev.md
Normal file
103
doc/Contributions/Pratiques-de-dev.md
Normal file
|
@ -0,0 +1,103 @@
|
|||
# Bonnes pratiques de développement
|
||||
|
||||
## Branches du projet
|
||||
|
||||
- `main`, branche qui contient le code du site tel qu'il est [en intégration](https://dev.demarches-simplifiees.fr/)
|
||||
- `production`, branche qui contient le code du site tel qu'il est [en production](https://www.demarches-simplifiees.fr)
|
||||
|
||||
## Cycle de développement
|
||||
|
||||
1. Pour chaque ensemble de modifications, créer une branche associée
|
||||
(sur le repo principal si vous avez les droits, sinon sur un fork personnel).
|
||||
2. Une fois les modifications faites, créer une Pull Request sur GitHub.
|
||||
3. Si un·e relectrice·eur ne s'est pas manifesté·e au bout de 48 h, relancer en mettant un message dans la PR.
|
||||
4. Quand la Pull Request a été relue et validée :
|
||||
1. le contributeur rebase la branche par rapport à `main` (manuellement, ou en mettant un commentaire "/rebase" dans la PR),
|
||||
2. le mainteneur merge la PR.
|
||||
|
||||
## Pour une première contribution
|
||||
|
||||
Si c’est votre première contribution, commencez par une toute petite Pull Request (PR), par exemple de nettoyage,
|
||||
pour vous faire la main sur le processus.
|
||||
|
||||
## Bonnes pratiques : sur le code
|
||||
|
||||
### Tests
|
||||
|
||||
- Le code doit être testé, la PR doit contenir les tests (toute PR sans test sera a priori rejetée).
|
||||
|
||||
### Code mort / code propre
|
||||
|
||||
- Une contribution ne devrait pas comporter de code mort (enlever le code commenté ou jamais appelé).
|
||||
|
||||
### Injection de dépendance
|
||||
|
||||
D'une manière générale, nous préférons avoir des controlleurs verbeux mais explicites.
|
||||
Afin d'éviter de trop alourdir les contrôleurs et les modèles, déjà bien chargés, nous mettons parfois en place des services pour centraliser une partie cohérente des traitements (voir [DossierProjectionService](https://github.com/betagouv/demarches-simplifiees.fr/blob/92f463bc039200b98908dc5c09366844b0e1d593/app/services/dossier_projection_service.rb), [PieceJustificativeService](https://github.com/betagouv/demarches-simplifiees.fr/blob/92f463bc039200b98908dc5c09366844b0e1d593/app/services/pieces_justificatives_service.rb))
|
||||
|
||||
- Toute injection de dépendance doit être utilisée (sinon ne pas la coder)
|
||||
- Il est demandé d'éviter l'injection de dépendance dans les constructeurs.
|
||||
(Une exception pourrait être prise en compte si deux implémentations différentes, hors tests, sont injectées.)
|
||||
- On mock directement les dépendances concernées (parce que ruby, c'est magique)
|
||||
|
||||
```ruby
|
||||
# Non
|
||||
def initializer(http_service)
|
||||
@http_service = http_service
|
||||
end
|
||||
|
||||
def get
|
||||
@http_service.get(url)
|
||||
end
|
||||
|
||||
# Oui
|
||||
def get
|
||||
Typhoeus.get(url)
|
||||
end
|
||||
|
||||
# spec
|
||||
|
||||
expect(Service).to receive(:do_stuff).and_return(true)
|
||||
## ou même
|
||||
expect_any_instance_of(Procedure).to receive(:procedure_overview).and_return(procedure_overview)
|
||||
```
|
||||
|
||||
## Bonnes pratiques : sur les PR
|
||||
|
||||
- Toujours mettre un message décrivant l'objet de la PR
|
||||
- Si la PR concerne des changements visuels, mettre une capture d'écran
|
||||
- Faites de petites PR. Si la PR est trop grosse (> 400 lignes), découpez-la en plusieurs PR. Chaque petite PR doit
|
||||
essayer d'apporter de la valeur au client, d'apporter une petite fonctionnalité.
|
||||
- Le relecteur d'une PR peut prendre la main (après l'avoir demandé) pour faire les modifs de formes
|
||||
- Les modifications de formes non détectées automatiquement sont optionnelles
|
||||
- Néanmoins, les commits de nettoyage sont autorisés au sein de ces petites PRs
|
||||
- Les remarques sur l’amélioration du code ne concernant pas directement la PR sont optionnelles
|
||||
|
||||
En date du 2021-10-19, voici une PR servant d'exemple :
|
||||
|
||||
* [#6519 ETQ Super Admin je veux changer le mail d'un instructeur](https://github.com/betagouv/demarches-simplifiees.fr/pull/6519)
|
||||
|
||||
## Bonnes pratiques : sur les branches
|
||||
|
||||
- Donner un nom descriptif à ses branches
|
||||
- Faire une branche par "sujet", ne pas faire de branches trop lourdes
|
||||
|
||||
**Attention** : Ne **pas utiliser le bouton "Update branch"** de GitHub.
|
||||
|
||||
Ce bouton merge `main` dans la feature branch – ce qui casse l'historique semi-linéraire. (Nous, ce qu'on voudrait, c'est rebaser).
|
||||
À la place, rebaser manuellement la feature-branch sur `main` (ou mettre un commentaire "/rebase" dans la PR).
|
||||
|
||||
## Bonnes pratiques : sur les commits
|
||||
|
||||
- Les messages de commit sont écrits en anglais
|
||||
- Faire des petits commits, les plus unitaires possible, homogènes et en essayant de ne pas mélanger les sujets.
|
||||
- Les commits correctifs sont à "fixup-er" dans les commits qu'ils corrigent
|
||||
- Séparer les modifications relatives à du nettoyage dans un commit séparé, voire une PR séparée
|
||||
- Dans le cas où un commit corrige un bug ou implémente une feature, mentionner dans le message de commit le numéro de l'issue avec `Fix #XXXX` ou `Ref #XXXX`
|
||||
|
||||
Exemple d'une série de commits :
|
||||
|
||||
- un commit pour du renommage,
|
||||
- un commit pour un ajout de méthode + test,
|
||||
- un commit pour l'interface utilisateur
|
||||
|
Loading…
Add table
Add a link
Reference in a new issue