Hi all. I've been trying to push as much as I can from the PQA queue, and I'm finding that most of the patches haven't been tidied up yet or have trivially caught issues (maybe not most, but a lot). For example, this is what I find when I run an up to date KTD and the code is not tidy: ``` kohadev-koha@kohadevbox:koha(master)$ qa -c 1 --run-tests testing 1 commit(s) (applied to e0df163 'fcf Bug 33933: Only show use restrict') Processing files before patches |========================>| 2 / 2 (100.00%) Processing files after patches |========================>| 2 / 2 (100.00%) WARN Koha/Recalls.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 41, now: 42) WARN t/db_dependent/Koha/Recalls.t WARN tidiness The file is less tidy than before (bad/messy lines before: 100, now: 101) Processing additional checks OK! Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/Koha/Recalls.t OK! ``` This is probably because they were submitted *before* we decided to enforce it on the QA tools, and it is understandable that, in some cases, it was not caught by a not-up-to-date QA command. As all testers and QA team members are encouraged to run the `qa` script before signing patches, I assume the underlying problem is that their KTD images are old and don't contain the latest QA tools version. So please make sure your KTD images or qa-test-tools clone is up to date. And run the tool, all the time! So, to sumarize: - Patch authors need to tidy the new blocks of code they introduce, or modify. - Instructions on setting perltidy on your favourite editor can be found on the wiki [1]. If your editor is missing, ask for help on IRC, the mailing list, or just use one from the documented ones. - QA members need to run the QA scripts. - You need to build the habit of pulling the latest KTD images every now and then. In case important things changed. And also run `docker system prune -a` (while KTD is running, so only the old images are cleared) - The RM will start failing patches that the QA script says have issues. Both authors and testers have the ability to run the script before submission. Every extra minute it takes to be gentle and fix the patch inline makes us have less time to push your next stuff! If you read all this, thank you! You are awesome! And I'm really glad to be working on this side-by-side with you! And I hope to share some beers and cookies in Helsinki soon! Cheers! [1] https://wiki.koha-community.org/wiki/Perltidy -- Tomás Cohen Arazi Theke Solutions (https://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
Great job, Tomas. Keep pushing; your queue is not empty yet 😉 As this is the first mention about requiring perl tidied code, I would assume that we will be enforcing it (as QA team) only on code that was submitted from now on. And we need some common sense too imo. We shouldnt reject patches only for adding or missing spaces around curly braces and the like. There will be good 'messy' code and tidy code containing terrible bugs. Marcel ________________________________ Van: Koha-devel <koha-devel-bounces@lists.koha-community.org> namens Tomas Cohen Arazi <tomascohen@gmail.com> Verzonden: donderdag 13 juli 2023 18:02 Aan: koha-devel <koha-devel@lists.koha-community.org> Onderwerp: [Koha-devel] Tidy your code! And run the QA script! Hi all. I've been trying to push as much as I can from the PQA queue, and I'm finding that most of the patches haven't been tidied up yet or have trivially caught issues (maybe not most, but a lot). For example, this is what I find when I run an up to date KTD and the code is not tidy: ``` kohadev-koha@kohadevbox:koha(master)$ qa -c 1 --run-tests testing 1 commit(s) (applied to e0df163 'fcf Bug 33933: Only show use restrict') Processing files before patches |========================>| 2 / 2 (100.00%) Processing files after patches |========================>| 2 / 2 (100.00%) WARN Koha/Recalls.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 41, now: 42) WARN t/db_dependent/Koha/Recalls.t WARN tidiness The file is less tidy than before (bad/messy lines before: 100, now: 101) Processing additional checks OK! Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/Koha/Recalls.t OK! ``` This is probably because they were submitted *before* we decided to enforce it on the QA tools, and it is understandable that, in some cases, it was not caught by a not-up-to-date QA command. As all testers and QA team members are encouraged to run the `qa` script before signing patches, I assume the underlying problem is that their KTD images are old and don't contain the latest QA tools version. So please make sure your KTD images or qa-test-tools clone is up to date. And run the tool, all the time! So, to sumarize: - Patch authors need to tidy the new blocks of code they introduce, or modify. - Instructions on setting perltidy on your favourite editor can be found on the wiki [1]. If your editor is missing, ask for help on IRC, the mailing list, or just use one from the documented ones. - QA members need to run the QA scripts. - You need to build the habit of pulling the latest KTD images every now and then. In case important things changed. And also run `docker system prune -a` (while KTD is running, so only the old images are cleared) - The RM will start failing patches that the QA script says have issues. Both authors and testers have the ability to run the script before submission. Every extra minute it takes to be gentle and fix the patch inline makes us have less time to push your next stuff! If you read all this, thank you! You are awesome! And I'm really glad to be working on this side-by-side with you! And I hope to share some beers and cookies in Helsinki soon! Cheers! [1] https://wiki.koha-community.org/wiki/Perltidy -- Tomás Cohen Arazi Theke Solutions (https://theke.io<https://theke.io/>) ✆ +54 9351 3513384 GPG: B2F3C15F
participants (2)
-
Marcel de Rooy -
Tomas Cohen Arazi