You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Donc avec du recul: le fichier github.js contient:
du code d'aiguillage des évènements Github processWebhook: c'est bien dans son scope de controlleur github
du code qui déclenche des actions suite à ces événements, par exemple appel Scalingo, avec un mapping 1:1 sur une évènement Github, ex: pullRequestOpenedWebhook: je pense que sa place est dans un autre fichier
du code utilisé par les précédents: leur place est avec les précédents.
Da ma vision:
1 fichier controller github - 1 export
1 fichier avec leur actions métier - 1 export par action métier
Ces deux fichiers sont testés en unitaire SANS exporter des fonctions utilisées par d'autres fonctions dans le même fichier et pas ailleurs. Cers fonctions sont des détails d'implémentation. Si on veut les tester parce qu'elles contiennent trop de code, c'est l'indice qu'elles ont droit à une chambre à elles = un fichier séparé
Cela dit, je partage tout de même une autre solution.
L'interface du fichier est
{
addMessageToPullRequest,
getMessage,
getMessageTemplate,
processWebhook,
pullRequestOpenedWebhook,
pullRequestSynchronizeWebhook,
pushOnDefaultBranchWebhook,
};
Seule cette fonction est utilisée (hors tests):
processWebhook
Les fonctions suivantes gèrent un évènement métier
Et celles-ci sont utilisées poar les précédentes
Donc avec du recul: le fichier
github.js
contient:processWebhook
: c'est bien dans son scope de controlleur githubpullRequestOpenedWebhook
: je pense que sa place est dans un autre fichierDa ma vision:
Ces deux fichiers sont testés en unitaire SANS exporter des fonctions utilisées par d'autres fonctions dans le même fichier et pas ailleurs. Cers fonctions sont des détails d'implémentation. Si on veut les tester parce qu'elles contiennent trop de code, c'est l'indice qu'elles ont droit à une chambre à elles = un fichier séparé
Originally posted by @octo-topi in #296 (review)
The text was updated successfully, but these errors were encountered: