[Koha-devel] Tidy your code! And run the QA script!

[Marcel de Rooy]

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 at lists.koha-community.org> namens Tomas Cohen Arazi <tomascohen at gmail.com>
Verzonden: donderdag 13 juli 2023 18:02
Aan: koha-devel <koha-devel at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20230714/b4fbec11/attachment-0001.htm>