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-boun...@lists.koha-community.org> namens Tomas 
Cohen Arazi <tomasco...@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
_______________________________________________
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/

Reply via email to