Le Wed, 7 Sep 2016 11:39:22 +0200 (CEST) Johan Cwiklinski <jcwiklin...@teclib.com> a écrit:
>Hello, > >----- Mail original ----- >> De: "David DURIEUX" <d.duri...@siprossii.com> >> À: "Liste de diffusion des developpeurs GLPI" <glpi-dev@gna.org> >> Envoyé: Mercredi 7 Septembre 2016 11:16:46 >> Objet: [Glpi-dev] Modify coding methods to enhance code quality >> >> Hello, >> >> I propose new coding methods to enhance GLPI (better code, have >> tests, so less bugs): >> >> * NEVER (so no exceptions) commit directly in the repository, always >> create Pull Requests > >I agree with that. > >> * All Pull Request must be linked with an issue (bugs, features...) > >Agree. > >> * All issues for features must be discussed and so made specification >> of what to do in the issue, with that, the developer will win time >> when will code it. The developer must be assigned to the issue > >Agree. > >> * All Pull Requests must be reviewed by another developer, this >> review is needed and check these points: >> * Check the code, if it's coherent with the issue, coding >> standards >> * check if there are enough tests (unit, integration). So a Pull >> Request NEED HAVE tests, if no test, refure merge and add >> comment in it >> * verify travis (run tests in travis-ci.org) pass > >Agree, but disagree (on the tests part) :p > >We should probably have tests on new features, but when it is actually >possible; if we need to refactor some other (big) parts of code to get >tests running, this is probably not a good idea ; and for bugfixes, we >probably cannot ask the developper to spend all the required time to >write all tests on some cases. No, the bug fix is as much important than test, so can't dissociate the both. >At the end, this rule will really become a MUST, but I'm not sure it >is possible right now (talking about exeprience on some other projets >where some parts of the code cannot be unit tested by design - or >mistakes). I can be wrong, I'm quite new to the project ;) > >On the basics, I totally agree, but maybe should we consider merging >without tests would be possible with a (really!) good reason for now? >Of course, existing unit tests MUST be OK, in any case. Of course if you have a Pull Request have only add comments in code, no tests needed :D >I'd like to add that developper that will take review MUST assign >him/herself to the PR. > >> * Once the PR is reviewed and all is OK, merge it. You DON'T MERGE >> for the developper pleasure, you merge because you think it's good >> for GLPI and code is good quality. > >Agree. > >> So yes, you think you will loose time, but if you have less bugs to >> fix after because it's verified by unit tests, you win time ;) >> >> Another point, for the release it will be better, you can release >> directly or perhaps have one RC. Release process is more simple and >> you can release when you want with these methods. >> >> I will do that in many projects (with big code and very few >> developers) and we win time, quality of code. > >Review process and automated tests are really a must have; I can't >disagree with that :) > >We'll also have to write a (clear and quite simple) page explaining >the whole process, for everyone reference. > >Just my two cents, >++ _______________________________________________ Glpi-dev mailing list Glpi-dev@gna.org https://mail.gna.org/listinfo/glpi-dev
