As said in the email I am planning to integrate them into our dev env (via misc4dev), see https://gitlab.com/koha-community/koha-misc4dev/-/issues/59
Le lun. 5 déc. 2022 à 15:17, Marcel de Rooy <m.de.r...@rijksmuseum.nl> a écrit : > > Thanks for sending this link. > Could we merge that code with QA code we already pushed into our codebase > somehow ? > > -----Original Message----- > From: Koha-devel <koha-devel-boun...@lists.koha-community.org> On Behalf Of > Jonathan Druart > Sent: Monday, December 5, 2022 2:56 PM > To: koha-devel <koha-devel@lists.koha-community.org> > Subject: Re: [Koha-devel] Good enough? > > Existing hooks can be found at > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwiki.koha-community.org%2Fwiki%2FTips_and_tricks%23Hooks&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bxdii78avAYEIcecc8o0tQZ4v7atwVTkmbJMYPe1Dek%3D&reserved=0 > > Le lun. 5 déc. 2022 à 11:34, Marcel de Rooy <m.de.r...@rijksmuseum.nl> a > écrit : > > > > Could you provide more info on those git hooks? A wiki URL? > > > > Marcel > > > > -----Original Message----- > > From: Koha-devel <koha-devel-boun...@lists.koha-community.org> On > > Behalf Of Jonathan Druart > > Sent: Friday, December 2, 2022 3:43 PM > > To: koha-devel <koha-devel@lists.koha-community.org> > > Subject: [Koha-devel] Good enough? > > > > Hi devs, > > > > I was wondering... How good is your "good enough"? > > It's a question I often ask myself, when writing patches or QAing > > yours: is it good enough to be into Koha? It does not have to be perfect or > > it may never reach master, but it must meet certain standards. > > > > When I was RM I tried to put that limit quite high. Not necessarily by > > asking the author for improving the follow-up patches, but also by adding > > the missing bits myself. > > > > There are various types of "good enough", depending on what we look > > at: good enough to be reviewed, good enough to be tested, to be put in > > production, get feedback, try an idea, etc. > > > > Most of the time, what is driving the limit is answering the questions "Is > > it maintainable?" / "Is it future proof?". Making sure the code you are > > writing won't be broken inadvertently is very important and must be part of > > the reflection. > > > > Katrin asked the QA team members what were our plans for 23.05. In my > > opinion we should enforce this "be future proof" concept. Writing code is > > easy, especially in Koha (yes it is!). Writing maintainable and robust > > code, following our guidelines is a bit harder. That's why we have a QA > > process that tries to catch inconsistencies or edge cases you may have > > missed. > > But I think we can be even more rigorous, and aim for better > > standards. We can elevate our ambitions and do better. When we see the > > number of new enhancements we are releasing every 6 months, it shows > > well that writing code is definitely not a problem. However sometimes > > developers are tempted to abandon their work once they are pushed to > > master. Pushed does not mean "done". Providing bug fixes, following-up > > with patches when needed, fixing CI jenkins, etc. is part of job > > (/fun) > > > > As a Koha developer for a long time now, I know how frustrating it can be > > to be asked for follow-ups/rewrite/tests to have our patches stamped with > > the precious PQA mark. But from the other point of view (RM, RMaints, QA > > team), I also know it's very frustrating when you are in charge of the > > release and you do not get the appropriate follow-up work once it's pushed > > to master. > > > > There are some easy steps to write/review better patches. All have > > been discussed already several times, but that can be enforced even > > more: > > 1. Perltidy (!) This is really a very trivial step. Please perltidy > > your code. There are hundreds of commits that have been pushed in the > > last months that are not tidy (alignment, indentation, lines too long, > > etc.) This can easily be configured in your IDE! [1] > > > > 2. Provide clean code. As said it's not necessarily easy, but the QA team > > and RM are supposed to know if the code is clean regarding Koha guidelines. > > If the code is not clean, don't PQA, don't push. Either clean yourself, or > > ask the original author of the patch to do it (explaining to them how it > > can be improved ofc). > > > > 3. Squash! I have been away for a couple of months and had to read the git > > history to know what I missed. And it was really hard to follow what was > > going on. First of all, we are not consistent: the commit message must tell > > what the patch is doing, not what the bug was (if you are writing a bug > > fix). Then, there are way too many follow-ups: > > tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups > > could be squashed into the original patch. We don't need unnecessary tons > > of entries in our git log for that. For instance, I usually add a "JD > > Amended patch: perltidy" for instance when I tidy the original patch, to > > keep track of the modification. Squash can be done by the original author, > > the QAer, the RM. So yes, you are losing one commit in the stats but the > > git log is easy to read! > > We could have an "Amended-by" marker if we really want to add credit on the > > dashboard (and/or release notes). > > > > 4. Run tests. Don't wait for Jenkins to fail. This is valid for the author > > and QA. Anticipate the failures by running more tests. If you are modifying > > C4::Circulation, then run prove on t/db_dependent/Circulation*, not only > > Circulation.t. It will help you catch edge cases. > > When something is pushed, track down jenkins failures that could be caused > > by your patches. > > > > 6. Be strict if you are QAing. Each QA member has their own "good enough", > > and the RM as well (either relying on the QAer or providing a full review). > > But QA must fail if the code is old Koha style code, or not "good enough". > > > > 7. Provide support for failing tests, fix things you broke. The QA team > > will be more comfortable with your patches if you show them you are > > providing support for your stuff. > > It's not because it's pushed that you don't have any more efforts to make. > > Provide follow-up patches you promised, provide bug fixes, etc. > > We don't have a good way to keep track of such demands, which does not make > > tracking easier for devs, QA and RM. Any suggestions? > > > > 8. QA team MUST NEVER* pass QA a change that is not covered by tests, > > never. You should not provide change to modules without tests! > > * almost never... > > > > 9. Stick to existing patterns. We should not have different ways to do the > > same thing. We should not have different places where a code is doing the > > same thing. Ask for help or advice on the list or IRC before you start > > coding. We will be happy to guide you. Even if you are a regular Koha > > developer it's not always easy to be aware of the latest master changes. > > We will tell you what's the current good practice, or point you to examples > > you could reuse for what you want to implement. > > > > 10. CI should drive the pushes. No more push if CI is not green. The more > > we wait the harder it is to track down the origin of the problem. > > Last cycle some jobs have been red for months, and we released > > 22.11.00 with D10, D11, D12 marked unstable... > > > > What will I do next cycle? > > All of that, and more. I will track down jenkins failures and > > responsibilize developers telling them when they break tests (and won't fix > > them anymore as I have been doing for years). > > I will raise on the bug reports what could have been improved. Yes, read > > that I will be even more annoying (to put it politely) than before. > > > > I've noticed that the pre-commit git hook on the wiki has been broken for > > more than 3 years. And also caught some core developers that do not have it > > in place. I am relying on it to keep Vue files tidy so it's important to > > have it set up properly. I am planning to force its usage for ktd users > > [2]. Adding more checks to it will help us to catch inconsistencies from > > the beginning. > > > > To summarize, writing code is cheap, maintaining code is way more > > expensive! It is easier to get the attention of developers before the > > patches are pushed to master than after, so we could be more ambitious and > > ask more. > > > > For discussion :) > > > > Cheers, > > Jonathan > > > > [1] If you are using vim, open ~/vimrc, add > > vmap <F8> :!perltidy -q<CR> > > Reload vim, select code in visual mode [2] > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl > > ab.com%2Fkoha-community%2Fkoha-misc4dev%2F-%2Fissues%2F59&data=05% > > 7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7 > > C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnkno > > wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=AkLM4JATxwG4L8CcLpZ7z34oM6GCEN > > 49uAo3dt4Sw18%3D&reserved=0 > > _______________________________________________ > > Koha-devel mailing list > > Koha-devel@lists.koha-community.org > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist > > s.koha-community.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fkoha-devel&d > > ata=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c > > 87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550% > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik > > 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=aqN6n2Z2kvsodCb1MlLsNiA > > AomgcvKr6fB7tCNNdBrg%3D&reserved=0 > > website : > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. > > koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C > > 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C > > 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA > > iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > ata=3vEx8LIuZAkid78L%2FF15H58ECkQQu4m4PO7R4Tnbot0%3D&reserved=0 > > git : > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C > > 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C > > 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA > > iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > ata=5bMenLRZpyj4HYRx%2FExY83Mwe%2F8bpaLvlJvG3jLcBCg%3D&reserved=0 > > bugs : > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs > > .koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7 > > C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7 > > C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s > > data=iT5Hl4dNO8NQXuQZmJmkdLr1pXv0k3TBGK8NceA82iE%3D&reserved=0 > _______________________________________________ > Koha-devel mailing list > Koha-devel@lists.koha-community.org > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha-community.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fkoha-devel&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=aqN6n2Z2kvsodCb1MlLsNiAAomgcvKr6fB7tCNNdBrg%3D&reserved=0 > website : > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3vEx8LIuZAkid78L%2FF15H58ECkQQu4m4PO7R4Tnbot0%3D&reserved=0 > git : > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5bMenLRZpyj4HYRx%2FExY83Mwe%2F8bpaLvlJvG3jLcBCg%3D&reserved=0 > bugs : > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iT5Hl4dNO8NQXuQZmJmkdLr1pXv0k3TBGK8NceA82iE%3D&reserved=0 > _______________________________________________ > Koha-devel mailing list > Koha-devel@lists.koha-community.org > https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel > website : https://www.koha-community.org/ > git : https://git.koha-community.org/ > bugs : https://bugs.koha-community.org/ _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
