Bug 39149 - .PL files not tidy
Summary: .PL files not tidy
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords: RM_priority
Depends on: 39096
Blocks:
  Show dependency treegraph
 
Reported: 2025-02-17 17:18 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-03-10 11:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39149: Make tidy.pl deal with .PL files (1.70 KB, patch)
2025-02-17 17:41 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39149: Tidy .PL files (94.01 KB, patch)
2025-02-17 17:41 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39149: Make tidy.pl deal with .PL files (1.75 KB, patch)
2025-02-17 19:13 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 39149: Tidy .PL files (94.06 KB, patch)
2025-02-17 19:14 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 39149: Make perltidy.t also check .PL files (917 bytes, patch)
2025-02-17 19:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 39149: Make perltidy.t also check .PL files (975 bytes, patch)
2025-02-18 11:02 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2025-02-17 17:18:49 UTC
Somehow we missed we have `.PL` files in the codebase. They are not considered in `tidy.pl` and remain untidy.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-02-17 17:41:28 UTC
Created attachment 178148 [details] [review]
Bug 39149: Make tidy.pl deal with .PL files

This patch adds handling for files with the `.PL` extension to the
`tidy.pl` helper script.

It will now consider them Perl files and tidy them as appropriate.

I opted for explicitly listing `PL` instead of making the existing check
case-insensitive because the only files I found with changed case (i.e.
Perl scripts that don't have `.pl`) are:

$ find . -type f -iname "*.pl" ! -name "*.pl"
./fix-perl-path.PL
./build-resources.PL
./rewrite-config.PL
./Makefile.PL

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2025-02-17 17:41:31 UTC
Created attachment 178149 [details] [review]
Bug 39149: Tidy .PL files

This patch is the result of running:

```shell
perl misc/devel/tidy.pl *.PL
```

Commiting raised an error about fix-perl-path.PL calling `warn` which I dismiss for the
time being.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Victor Grousset/tuxayo 2025-02-17 18:10:44 UTC
Good news, it seems there aren't other cases of files with slightly diverging extensions that escaped tidying.

After looking at all the stuff not having the most common extensions:
find . -type f -not -path "./.git/*" -not -path "./misc/translator/po/.git/*" -regextype posix-egrep -not -regex ".*\.(sql|pl|yml|md|yaml|js|txt|xml|json|xsl|chr|pm|t|ts|mrc|csv|inc|pod|att|tag|abs|mar|cfg|conf|properties|ods|po|png|svg|idx|gif|tt|css|vue|html|jpg|in|ogg|woff|woff2|ttf|map|scss|map|py|pref|sh|pdf|odt)$"
Comment 4 Victor Grousset/tuxayo 2025-02-17 19:13:56 UTC
Created attachment 178150 [details] [review]
Bug 39149: Make tidy.pl deal with .PL files

This patch adds handling for files with the `.PL` extension to the
`tidy.pl` helper script.

It will now consider them Perl files and tidy them as appropriate.

I opted for explicitly listing `PL` instead of making the existing check
case-insensitive because the only files I found with changed case (i.e.
Perl scripts that don't have `.pl`) are:

$ find . -type f -iname "*.pl" ! -name "*.pl"
./fix-perl-path.PL
./build-resources.PL
./rewrite-config.PL
./Makefile.PL

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2025-02-17 19:14:00 UTC
Created attachment 178151 [details] [review]
Bug 39149: Tidy .PL files

This patch is the result of running:

```shell
perl misc/devel/tidy.pl *.PL
```

Commiting raised an error about fix-perl-path.PL calling `warn` which I dismiss for the
time being.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2025-02-17 19:15:04 UTC
It works! :)

Testing notes: apply last commit of Bug 39096 before this, otherwise it won't apply.
Because that follow-up isn't on main yet.
Bug 39096: Add .git-blame-ignore-revs mapping to Makefile.PL
Comment 7 Victor Grousset/tuxayo 2025-02-17 19:41:01 UTC
Created attachment 178152 [details] [review]
Bug 39149: Make perltidy.t also check .PL files

Now perltidy.t will check 2958 files instead of 2954. Which matches
with the 4 .PL files.
Comment 8 Tomás Cohen Arazi (tcohen) 2025-02-18 11:02:44 UTC
Created attachment 178170 [details] [review]
Bug 39149: Make perltidy.t also check .PL files

Now perltidy.t will check 2958 files instead of 2954. Which matches
with the 4 .PL files.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Katrin Fischer 2025-02-18 14:36:20 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 10 Jonathan Druart 2025-03-10 11:38:05 UTC
Added a commit to koha-testing-docker for the pre-commit git hooks

https://gitlab.com/koha-community/koha-testing-docker/-/commit/d2b480e2c2462b155153d57bf246de20c4481266