30,909
edits
DevCamp presentation: PR to merge or not to merge: Difference between revisions
Jump to navigation
Jump to search
m
DevCamp presentation: PR to merge or not to merge (view source)
Revision as of 09:43, 10 June 2024
, 2 years ago→Quelques exemples
Tag: 2017 source edit |
|||
| Line 117: | Line 117: | ||
*New trigger CRONJOB_ERROR | *New trigger CRONJOB_ERROR | ||
*ici on a une mauvaise utilisation des triggers et il aurait fallu utiliser des hooks | *ici on a une mauvaise utilisation des triggers et il aurait fallu utiliser des hooks. Voir [Système_de_Hooks] et [Système_de_Triggers]. | ||
*le choix du trigger aurait été valable si on avait des transactions avec un test de retour pour savoir si on "commit" ou "rollback" les données | *le choix du trigger aurait été valable si on avait des transactions avec un test de retour pour savoir si on "commit" ou "rollback" les données et si l'action était de type CRUD (Create/Read/Update/Delete) d'un objet métier. | ||
=> on ne merge pas et il faudrait proposer une nouvelle PR avec l'utilisation de hooks | => on ne merge pas et il faudrait proposer une nouvelle PR avec l'utilisation de hooks | ||
| Line 126: | Line 126: | ||
<nowiki>https://github.com/Dolibarr/dolibarr/pull/29749/</nowiki> | <nowiki>https://github.com/Dolibarr/dolibarr/pull/29749/</nowiki> | ||
*ici on a un paquet de lignes | *ici on a un paquet de lignes dont le rôle est difficile à déterminer à vue d'oeil. | ||
=> au final il faut plus de commentaires de la part du développeur | => au final il faut plus de commentaires de la part du développeur | ||