Hello, I agree with the whole, and just want to add 2 things: 1) for the assignments: it can be difficult for outside developers (like me). 2) for the tests, we should have unitary tests, but also integration tests , and at least functional tests. And for me these functional tests should be documented by the developer (or tester) as they are mainly done manually.
Regards, Tomolimo -----Original Message----- From: Glpi-dev [mailto:glpi-dev-boun...@gna.org] On Behalf Of Johan Cwiklinski Sent: Wednesday, September 07, 2016 11:39 AM To: Liste de diffusion des developpeurs GLPI Subject: Re: [Glpi-dev] Modify coding methods to enhance code quality 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. 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. 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, ++ -- Johan _______________________________________________ Glpi-dev mailing list Glpi-dev@gna.org https://mail.gna.org/listinfo/glpi-dev _______________________________________________ Glpi-dev mailing list Glpi-dev@gna.org https://mail.gna.org/listinfo/glpi-dev
