104 lines
4.7 KiB
Markdown
104 lines
4.7 KiB
Markdown
|
# 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
|
|||
|
|
|