<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body>A new request with request id 6952 has been created by koha-devel-request@lists.koha-community.org. Short info on the request is : <br><br>Title : Koha-devel Digest, Vol 205, Issue 9<br>Category : <br>Description : <div>Send Koha-devel mailing list submissions to<br> koha-devel@lists.koha-community.org<br><br>To subscribe or unsubscribe via the World Wide Web, visit<br> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>or, via email, send a message with subject or body 'help' to<br> koha-devel-request@lists.koha-community.org<br><br>You can reach the person managing the list at<br> koha-devel-owner@lists.koha-community.org<br><br>When replying, please edit your Subject line so it is more specific<br>than "Re: Contents of Koha-devel digest..."<br><br><br>Today's Topics:<br><br> 1. Re: Good enough? (Jonathan Druart)<br><br><br>----------------------------------------------------------------------<br><br>Message: 1<br>Date: Mon, 5 Dec 2022 18:40:20 +0100<br>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org><br>To: koha-devel <koha-devel@lists.koha-community.org><br>Subject: Re: [Koha-devel] Good enough?<br>Message-ID:<br> <CAJzKNY7CAA4Tpx8PLZBhH2LQ3ONyS0xVQ1T=QmvP6M1ALYnJfg@mail.gmail.com><br>Content-Type: text/plain; charset="utf-8"<br><br>Having the hooks installed automatically will help devs catch small<br>inconsistencies, that's a first step for the easy-to-catch things.<br><br>The run_tests.pl script in misc4dev will hopefully help to run the tests by<br>anybody.<br><br>Running the tests on a DB without the sample data or with existing data is<br>already a goal, it's something we are trying to catch during QA.<br>TestBuilder helped us a lot for that. Create the data the tests need and<br>the cleanup/rollback should be the way to write tests.<br><br>Le lun. 5 déc. 2022 à 18:24, Julian Maurice <julian.maurice@biblibre.com> a<br>écrit :<br><br>> Yes it is expensive. But manual testing is much more expensive. I'd<br>> rather pay for some CPU time than do manually what a bot can do better<br>> and faster than me. Free services exist too (CircleCI, github actions, ...)<br>><br>> Pushing to a temporary master branch is not a bad idea (a never-broken<br>> master branch sounds nice), but I think it would happen too late.<br>> Patches would have already been rebased multiple times, tested and<br>> reviewed before we notice a test failure.<br>><br>> koha-testing-docker feels more like a symptom of the difficulty to run<br>> tests than a satisfying solution. It is probably necessary in order to<br>> run complicated end-to-end tests, but it should not be mandatory to run<br>> simple unit tests.<br>> And ktd is not that easy to use. It can break, and it's not easy to<br>> debug for someone not familiar with docker.<br>><br>> Don't you think it would be a lot easier if we could run `prove t` (or<br>> `npm test` or whatever) on any dev setup and have the same<br>> failures/successes as Jenkins (minus the complicated tests like those<br>> that require selenium) ?<br>> Of course making that happen is not an easy task, but it should be a<br>> long term goal IMO.<br>><br>> Also I think no one wants to be the person that refuse a patch because a<br>> comment is misaligned, so if that kind of thing is not automated, "not<br>> good enough code" will continue to be pushed.<br>><br>><br>> Le 05/12/2022 à 14:55, Jonathan Druart a écrit :<br>> > I don't think we should run the whole test suite everytime we attach<br>> > patches, that would be very expensive in terms of resources.<br>> > However it would be interesting to have a temporary 'master' branch<br>> > that would become 'master' only if jenkins is happy.<br>> > "master" would never be broken :D<br>> ><br>> > Using koha-testing-docker it's very easy to run tests locally.<br>> > It will be even more easier with<br>> > https://gitlab.com/koha-community/koha-misc4dev/-/issues/58<br>> ><br>> > Le ven. 2 déc. 2022 à 18:16, Julian Maurice<br>> > <julian.maurice@biblibre.com> a écrit :<br>> >><br>> >> Hi,<br>> >><br>> >> It seems to me that several issues could be solved by having the CI run<br>> >> sooner, so authors can have feedback as soon as possible. Something like<br>> >> github's CI integration with pull requests would be amazing to detect<br>> >> common mistakes early and stop wasting human time. We should know if<br>> >> tests pass before pushing to master, not after.<br>> >> We could check tidyness, commit message conventions, code coverage by<br>> >> tests, ... all before another person have to look at it.<br>> >><br>> >> Also tests are not easy to run locally. They might pass on Jenkins, but<br>> >> they do not on my local setup, so they are basically useless to tell me<br>> >> if I broke something. It also makes writing new tests more difficult<br>> >> than it should.<br>> >><br>> >> If I wanted to improve Koha developer experience, I would start with<br>> that.<br>> >><br>> >> My 2 cents.<br>> >><br>> >> Le 02/12/2022 à 15:42, Jonathan Druart a écrit :<br>> >>> Hi devs,<br>> >>><br>> >>> I was wondering... How good is your "good enough"?<br>> >>> It's a question I often ask myself, when writing patches or QAing<br>> >>> yours: is it good enough to be into Koha? It does not have to be<br>> >>> perfect or it may never reach master, but it must meet certain<br>> >>> standards.<br>> >>><br>> >>> When I was RM I tried to put that limit quite high. Not necessarily by<br>> >>> asking the author for improving the follow-up patches, but also by<br>> >>> adding the missing bits myself.<br>> >>><br>> >>> There are various types of "good enough", depending on what we look<br>> >>> at: good enough to be reviewed, good enough to be tested, to be put in<br>> >>> production, get feedback, try an idea, etc.<br>> >>><br>> >>> Most of the time, what is driving the limit is answering the<br>> >>> questions "Is it maintainable?" / "Is it future proof?". Making sure<br>> >>> the code you are writing won't be broken inadvertently is very<br>> >>> important and must be part of the reflection.<br>> >>><br>> >>> Katrin asked the QA team members what were our plans for 23.05. In my<br>> >>> opinion we should enforce this "be future proof" concept. Writing code<br>> >>> is easy, especially in Koha (yes it is!). Writing maintainable and<br>> >>> robust code, following our guidelines is a bit harder. That's why we<br>> >>> have a QA process that tries to catch inconsistencies or edge cases<br>> >>> you may have missed.<br>> >>> But I think we can be even more rigorous, and aim for better<br>> >>> standards. We can elevate our ambitions and do better. When we see the<br>> >>> number of new enhancements we are releasing every 6 months, it shows<br>> >>> well that writing code is definitely not a problem. However sometimes<br>> >>> developers are tempted to abandon their work once they are pushed to<br>> >>> master. Pushed does not mean "done". Providing bug fixes, following-up<br>> >>> with patches when needed, fixing CI jenkins, etc. is part of job<br>> >>> (/fun)<br>> >>><br>> >>> As a Koha developer for a long time now, I know how frustrating it can<br>> >>> be to be asked for follow-ups/rewrite/tests to have our patches<br>> >>> stamped with the precious PQA mark. But from the other point of view<br>> >>> (RM, RMaints, QA team), I also know it's very frustrating when you are<br>> >>> in charge of the release and you do not get the appropriate follow-up<br>> >>> work once it's pushed to master.<br>> >>><br>> >>> There are some easy steps to write/review better patches. All have<br>> >>> been discussed already several times, but that can be enforced even<br>> >>> more:<br>> >>> 1. Perltidy (!) This is really a very trivial step. Please perltidy<br>> >>> your code. There are hundreds of commits that have been pushed in the<br>> >>> last months that are not tidy (alignment, indentation, lines too long,<br>> >>> etc.) This can easily be configured in your IDE! [1]<br>> >>><br>> >>> 2. Provide clean code. As said it's not necessarily easy, but the QA<br>> >>> team and RM are supposed to know if the code is clean regarding Koha<br>> >>> guidelines. If the code is not clean, don't PQA, don't push. Either<br>> >>> clean yourself, or ask the original author of the patch to do it<br>> >>> (explaining to them how it can be improved ofc).<br>> >>><br>> >>> 3. Squash! I have been away for a couple of months and had to read the<br>> >>> git history to know what I missed. And it was really hard to follow<br>> >>> what was going on. First of all, we are not consistent: the commit<br>> >>> message must tell what the patch is doing, not what the bug was (if<br>> >>> you are writing a bug fix). Then, there are way too many follow-ups:<br>> >>> tidiness, indentation fix, typo, spelling, etc. All those tiny<br>> >>> follow-ups could be squashed into the original patch. We don't need<br>> >>> unnecessary tons of entries in our git log for that. For instance, I<br>> >>> usually add a "JD Amended patch: perltidy" for instance when I tidy<br>> >>> the original patch, to keep track of the modification. Squash can be<br>> >>> done by the original author, the QAer, the RM. So yes, you are losing<br>> >>> one commit in the stats but the git log is easy to read!<br>> >>> We could have an "Amended-by" marker if we really want to add credit<br>> >>> on the dashboard (and/or release notes).<br>> >>><br>> >>> 4. Run tests. Don't wait for Jenkins to fail. This is valid for the<br>> >>> author and QA. Anticipate the failures by running more tests. If you<br>> >>> are modifying C4::Circulation, then run prove on<br>> >>> t/db_dependent/Circulation*, not only Circulation.t. It will help you<br>> >>> catch edge cases.<br>> >>> When something is pushed, track down jenkins failures that could be<br>> >>> caused by your patches.<br>> >>><br>> >>> 6. Be strict if you are QAing. Each QA member has their own "good<br>> >>> enough", and the RM as well (either relying on the QAer or providing a<br>> >>> full review). But QA must fail if the code is old Koha style code, or<br>> >>> not "good enough".<br>> >>><br>> >>> 7. Provide support for failing tests, fix things you broke. The QA<br>> >>> team will be more comfortable with your patches if you show them you<br>> >>> are providing support for your stuff.<br>> >>> It's not because it's pushed that you don't have any more efforts to<br>> >>> make. Provide follow-up patches you promised, provide bug fixes, etc.<br>> >>> We don't have a good way to keep track of such demands, which does not<br>> >>> make tracking easier for devs, QA and RM. Any suggestions?<br>> >>><br>> >>> 8. QA team MUST NEVER* pass QA a change that is not covered by tests,<br>> >>> never. You should not provide change to modules without tests!<br>> >>> * almost never...<br>> >>><br>> >>> 9. Stick to existing patterns. We should not have different ways to do<br>> >>> the same thing. We should not have different places where a code is<br>> >>> doing the same thing. Ask for help or advice on the list or IRC before<br>> >>> you start coding. We will be happy to guide you. Even if you are a<br>> >>> regular Koha developer it's not always easy to be aware of the latest<br>> >>> master changes.<br>> >>> We will tell you what's the current good practice, or point you to<br>> >>> examples you could reuse for what you want to implement.<br>> >>><br>> >>> 10. CI should drive the pushes. No more push if CI is not green. The<br>> >>> more we wait the harder it is to track down the origin of the problem.<br>> >>> Last cycle some jobs have been red for months, and we released<br>> >>> 22.11.00 with D10, D11, D12 marked unstable...<br>> >>><br>> >>> What will I do next cycle?<br>> >>> All of that, and more. I will track down jenkins failures and<br>> >>> responsibilize developers telling them when they break tests (and<br>> >>> won't fix them anymore as I have been doing for years).<br>> >>> I will raise on the bug reports what could have been improved. Yes,<br>> >>> read that I will be even more annoying (to put it politely) than<br>> >>> before.<br>> >>><br>> >>> I've noticed that the pre-commit git hook on the wiki has been broken<br>> >>> for more than 3 years. And also caught some core developers that do<br>> >>> not have it in place. I am relying on it to keep Vue files tidy so<br>> >>> it's important to have it set up properly. I am planning to force its<br>> >>> usage for ktd users [2]. Adding more checks to it will help us to<br>> >>> catch inconsistencies from the beginning.<br>> >>><br>> >>> To summarize, writing code is cheap, maintaining code is way more<br>> >>> expensive! It is easier to get the attention of developers before the<br>> >>> patches are pushed to master than after, so we could be more ambitious<br>> >>> and ask more.<br>> >>><br>> >>> For discussion :)<br>> >>><br>> >>> Cheers,<br>> >>> Jonathan<br>> >>><br>> >>> [1] If you are using vim, open ~/vimrc, add<br>> >>> vmap <F8> :!perltidy -q<CR><br>> >>> Reload vim, select code in visual mode<br>> >>> [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59<br>> >>> _______________________________________________<br>> >>> Koha-devel mailing list<br>> >>> Koha-devel@lists.koha-community.org<br>> >>> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>> >>> website : https://www.koha-community.org/<br>> >>> git : https://git.koha-community.org/<br>> >>> bugs : https://bugs.koha-community.org/<br>> >><br>> >> --<br>> >> Julian Maurice<br>> >> BibLibre<br>> >> _______________________________________________<br>> >> Koha-devel mailing list<br>> >> Koha-devel@lists.koha-community.org<br>> >> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>> >> website : https://www.koha-community.org/<br>> >> git : https://git.koha-community.org/<br>> >> bugs : https://bugs.koha-community.org/<br>> > _______________________________________________<br>> > Koha-devel mailing list<br>> > Koha-devel@lists.koha-community.org<br>> > https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>> > website : https://www.koha-community.org/<br>> > git : https://git.koha-community.org/<br>> > bugs : https://bugs.koha-community.org/<br>><br>> --<br>> Julian Maurice<br>> BibLibre<br>> _______________________________________________<br>> Koha-devel mailing list<br>> Koha-devel@lists.koha-community.org<br>> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>> website : https://www.koha-community.org/<br>> git : https://git.koha-community.org/<br>> bugs : https://bugs.koha-community.org/<br>><br>-------------- next part --------------<br>An HTML attachment was scrubbed...<br>URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20221205/54bdfcdf/attachment.htm><br><br>------------------------------<br><br>Subject: Digest Footer<br><br>_______________________________________________<br>Koha-devel mailing list<br>Koha-devel@lists.koha-community.org<br>https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<br>website : https://www.koha-community.org/<br>git : https://git.koha-community.org/<br>bugs : https://bugs.koha-community.org/<br><br><br>------------------------------<br><br>End of Koha-devel Digest, Vol 205, Issue 9<br>******************************************<br></div><br><br>NOTE: You are receiving this mail because, the Requester/Technician wanted you to get notified on this request creation.<br></body></html>