Codespell is run by the QA script, but we have a lot of typos.
Created attachment 179269 [details] [review] Bug 39325: Automatic fix
Created attachment 179270 [details] [review] Bug 39325: [DO NOT PUSH] script Script used to generate the "Automatic fix" patch. I don't think we need this script pushed.
Created attachment 179271 [details] [review] Bug 39325: .codespell-ignore List of words to ignore
Created attachment 179272 [details] [review] Bug 39325: Add test We certainly want to improve this test and run it in parallel.
Anybody willing to help here? We need to review the "Automatic fix" commit and: 1. Make sure the changes are correct 2. Adjust the "FIXME CODESPELL" occurrences 3. Add words to .codespell-ignore when needed Unfortunately we cannot use the "inline ignore" (yet) that is in 2.3.0 that will be available in trixie (we have 2.2.2 in bookworm). https://github.com/codespell-project/codespell/pull/2400
Created attachment 179501 [details] [review] Bug 39325: [DO NOT PUSH] script Script used to generate the "Automatic fix" patch. I don't think we need this script pushed.
Created attachment 179502 [details] [review] Bug 39325: .codespell-ignore List of words to ignore
Created attachment 179503 [details] [review] Bug 39325: Add test We certainly want to improve this test and run it in parallel.
Created attachment 179504 [details] [review] Bug 39325: Automatic fix
Created attachment 179505 [details] [review] Bug 39325: Manual fixes
Created attachment 179506 [details] [review] Bug 39325: manual - reserv => reserve because of automatic changes in circ/transferstoreceive.tt
Created attachment 179507 [details] [review] Bug 39325: manual - hilighted => highlighted All the other occurrences have been updated.
Created attachment 179508 [details] [review] Bug 39325: manual - suggestion - no one => no_one It has been changed (automatically) from "noone" to "no one", which was wrong.
Created attachment 179509 [details] [review] Bug 39325: Adjust DB We need to adjust kohastructure.sql, sysprefs.sql and the DB itself.
Created attachment 179510 [details] [review] Bug 39325: Revert dbic schema files We need the md5sum to be adjusted. This should be done by the RM.
Created attachment 179511 [details] [review] Bug 39325: Tidy Some files need to be tidy after the automatic commit
Created attachment 179512 [details] [review] Bug 39325: Exclude atomic update - TODO RM TODO: We will need to adjust this path with the db_rev one
Several "words" have been added to .codespell-ignore in order to make the codespell tests pass. In a next step we will want to use the inline ignore. See previous comment: """ Unfortunately we cannot use the "inline ignore" (yet) that is in 2.3.0 that will be available in trixie (we have 2.2.2 in bookworm). https://github.com/codespell-project/codespell/pull/2400 """ Some manual changes are a bit messy (installer/data/mysql/db_revs/220600059.pl) to trick codespell, but overall the automatic changes are correct and everything was easy to deal with. Test plan: 1. Apply the whole patchset, and the dependencies (if not pushed yet). There is a remote branch available in my gitlab repo, with everything applied: https://gitlab.com/joubu/Koha/-/commits/bug_39325 2. Squash the "Automatic fix" and the manual patches for easy review 3. Have a look at the generated commit and review every changes 4. Run `prove xt/author_codespell.t`. The only failures should be on Koha/Schema/Result files. 5. Run the updatedatabase script and confirm that the typos have been fixed in the DB (mostly in the COMMENTs) Bonus, test the new version of the QA script 6. inside ktd, go to /kohadevbox/qa-test-tools and checkout the dedicated branch # as root % git remote add joubu https://gitlab.com/joubu/qa-test-tools.git % git checkout joubu/issue_93 # as koha user, go to /kohadevbox/koha and run the qa script % qa RM note: You will need to adjust the patch "Bug 39325: Exclude atomic update - TODO RM": the atomicupdate path needs to be the one of the db_rev, so the tests will ignore the file as it contains typo on purpose. Also don't forget to regenerate the dbic schema files. Several "words" have been added to .codespell-ignore to make the codespell tests pass. In the next step, we will use inline ignores. See the previous comment: """ Unfortunately, we cannot use the "inline ignore" feature (yet), which was introduced in version 2.3.0 and will be available in Trixie (we currently have 2.2.2 in Bookworm). https://github.com/codespell-project/codespell/pull/2400 """" Some manual changes are a bit messy (e.g., installer/data/mysql/db_revs/220600059.pl) as a workaround for codespell, but overall, the automatic changes are correct and easy to handle. Test Plan: 1. Apply the entire patch set, along with its dependencies (if they haven't been pushed yet). A remote branch with everything applied is available in my GitLab repo: https://gitlab.com/joubu/Koha/-/commits/bug_39325 2. Squash the "Automatic fix" and manual patches for easier review. 3. Review the generated commit and verify all changes. 4.Run prove xt/author_codespell.t. The only expected failures should be in Koha/Schema/Result files. 5.Run the updatedatabase script and confirm that the typos have been fixed in the database (mostly in the COMMENT fields). Bonus: 6. Test the new version of the QA script Inside ktd, navigate to /kohadevbox/qa-test-tools and check out the dedicated branch: # As root: % git remote add joubu https://gitlab.com/joubu/qa-test-tools.git % git checkout joubu/issue_93 # As the koha user, navigate to /kohadevbox/koha and run the QA script: % qa RM Note: You will need to adjust the patch "Bug 39325: Exclude atomic update - TODO RM". The atomicupdate path must match the corresponding db_rev, so the tests correctly ignore the file (as it contains intentional typos). Also, don’t forget to regenerate the DBIC schema files.
Several "words" have been added to .codespell-ignore to make the codespell tests pass. In the next step, we will use inline ignores. See the previous comment: """ Unfortunately, we cannot use the "inline ignore" feature (yet), which was introduced in version 2.3.0 and will be available in Trixie (we currently have 2.2.2 in Bookworm). https://github.com/codespell-project/codespell/pull/2400 """" Some manual changes are a bit messy (e.g., installer/data/mysql/db_revs/220600059.pl) as a workaround for codespell, but overall, the automatic changes are correct and easy to handle. Test Plan: 1. Apply the entire patch set, along with its dependencies (if they haven't been pushed yet). A remote branch with everything applied is available in my GitLab repo: https://gitlab.com/joubu/Koha/-/commits/bug_39325 2. Squash the "Automatic fix" and manual patches for easier review. 3. Review the generated commit and verify all changes. 4.Run prove xt/author_codespell.t. The only expected failures should be in Koha/Schema/Result files. 5.Run the updatedatabase script and confirm that the typos have been fixed in the database (mostly in the COMMENT fields). Bonus: 6. Test the new version of the QA script Inside ktd, navigate to /kohadevbox/qa-test-tools and check out the dedicated branch: # As root: % git remote add joubu https://gitlab.com/joubu/qa-test-tools.git % git checkout joubu/issue_93 # As the koha user, navigate to /kohadevbox/koha and run the QA script: % qa RM Note: You will need to adjust the patch "Bug 39325: Exclude atomic update - TODO RM". The atomicupdate path must match the corresponding db_rev, so the tests correctly ignore the file (as it contains intentional typos). Also, don’t forget to regenerate the DBIC schema files.
Created attachment 179517 [details] [review] Bug 39325: [DO NOT PUSH] script Script used to generate the "Automatic fix" patch. I don't think we need this script pushed. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179518 [details] [review] Bug 39325: .codespell-ignore List of words to ignore Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179519 [details] [review] Bug 39325: Add test We certainly want to improve this test and run it in parallel. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179522 [details] [review] Bug 39325: Automatic fix Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179525 [details] [review] Bug 39325: Manual fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179527 [details] [review] Bug 39325: manual - reserv => reserve because of automatic changes in circ/transferstoreceive.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179528 [details] [review] Bug 39325: manual - hilighted => highlighted All the other occurrences have been updated. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179529 [details] [review] Bug 39325: manual - suggestion - no one => no_one It has been changed (automatically) from "noone" to "no one", which was wrong. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179530 [details] [review] Bug 39325: Adjust DB We need to adjust kohastructure.sql, sysprefs.sql and the DB itself. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179531 [details] [review] Bug 39325: Revert dbic schema files We need the md5sum to be adjusted. This should be done by the RM. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179532 [details] [review] Bug 39325: Tidy Some files need to be tidy after the automatic commit Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179533 [details] [review] Bug 39325: Exclude atomic update - TODO RM TODO: We will need to adjust this path with the db_rev one Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 179534 [details] [review] Bug 39325: (follow-up) A few minor corrections
*** Bug 39458 has been marked as a duplicate of this bug. ***
Created attachment 180137 [details] [review] Bug 39325: [DO NOT PUSH] script Script used to generate the "Automatic fix" patch. I don't think we need this script pushed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180138 [details] [review] Bug 39325: .codespell-ignore List of words to ignore Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180139 [details] [review] Bug 39325: Add test We certainly want to improve this test and run it in parallel. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180140 [details] [review] Bug 39325: Automatic fix Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180141 [details] [review] Bug 39325: Manual fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180142 [details] [review] Bug 39325: manual - reserv => reserve because of automatic changes in circ/transferstoreceive.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180143 [details] [review] Bug 39325: manual - hilighted => highlighted All the other occurrences have been updated. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180144 [details] [review] Bug 39325: manual - suggestion - no one => no_one It has been changed (automatically) from "noone" to "no one", which was wrong. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180145 [details] [review] Bug 39325: Adjust DB We need to adjust kohastructure.sql, sysprefs.sql and the DB itself. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180146 [details] [review] Bug 39325: Revert dbic schema files We need the md5sum to be adjusted. This should be done by the RM. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180147 [details] [review] Bug 39325: Tidy Some files need to be tidy after the automatic commit Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180148 [details] [review] Bug 39325: Exclude atomic update - TODO RM TODO: We will need to adjust this path with the db_rev one Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180149 [details] [review] Bug 39325: (follow-up) A few minor corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 180150 [details] [review] Bug 39325: (QA follow-up) Adjust some issues connexion is a product, added to ignore file tablee -> tables a few other small corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 39325: [DO NOT PUSH] script Applying: Bug 39325: .codespell-ignore Applying: Bug 39325: Add test Applying: Bug 39325: Automatic fix .git/rebase-apply/patch:585: trailing whitespace. C<$needsconfirmation> a reference to a hash. It contains reasons why the loan .git/rebase-apply/patch:882: trailing whitespace. selects which template to use, either the opac or the intranet .git/rebase-apply/patch:2515: trailing whitespace. =cut .git/rebase-apply/patch:3756: trailing whitespace. =item cloud-kw.pl [--verbose|--help] --conf=F<cloud.conf> error: sha1 information is lacking or useless (C4/Languages.pm). error: could not build fake ancestor Patch failed at 0001 Bug 39325: Automatic fix hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-39325-Automatic-fix-_xnhnnpe.patch
Created attachment 181510 [details] [review] Bug 39325: [DO NOT PUSH] script Script used to generate the "Automatic fix" patch. I don't think we need this script pushed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181511 [details] [review] Bug 39325: .codespell-ignore List of words to ignore Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181512 [details] [review] Bug 39325: Add test We certainly want to improve this test and run it in parallel. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181513 [details] [review] Bug 39325: Automatic fix Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181514 [details] [review] Bug 39325: Manual fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181515 [details] [review] Bug 39325: manual - reserv => reserve because of automatic changes in circ/transferstoreceive.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181516 [details] [review] Bug 39325: manual - hilighted => highlighted All the other occurrences have been updated. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181517 [details] [review] Bug 39325: manual - suggestion - no one => no_one It has been changed (automatically) from "noone" to "no one", which was wrong. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181518 [details] [review] Bug 39325: Adjust DB We need to adjust kohastructure.sql, sysprefs.sql and the DB itself. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181519 [details] [review] Bug 39325: Revert dbic schema files We need the md5sum to be adjusted. This should be done by the RM. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181520 [details] [review] Bug 39325: Tidy Some files need to be tidy after the automatic commit Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181521 [details] [review] Bug 39325: Exclude atomic update - TODO RM TODO: We will need to adjust this path with the db_rev one Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181522 [details] [review] Bug 39325: (follow-up) A few minor corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181523 [details] [review] Bug 39325: (QA follow-up) Adjust some issues connexion is a product, added to ignore file tablee -> tables a few other small corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I am sorry, I already got a conflict again :( Can you please also check: I tried to leave off the first DO NOT PUSH patch (git bz apply -i), but then I also get a conflict, so I think some later patch also touches it... Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 39325: [DO NOT PUSH] script Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 39325: .codespell-ignore Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 39325: Add test Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 39325: Automatic fix .git/rebase-apply/patch:585: trailing whitespace. C<$needsconfirmation> a reference to a hash. It contains reasons why the loan .git/rebase-apply/patch:882: trailing whitespace. selects which template to use, either the opac or the intranet .git/rebase-apply/patch:2515: trailing whitespace. =cut .git/rebase-apply/patch:3763: trailing whitespace. =item cloud-kw.pl [--verbose|--help] --conf=F<cloud.conf> warning: 4 lines add whitespace errors. Using index info to reconstruct a base tree... M Makefile.PL M koha-tmpl/opac-tmpl/bootstrap/js/datatables.js .git/rebase-apply/patch:585: trailing whitespace. C<$needsconfirmation> a reference to a hash. It contains reasons why the loan .git/rebase-apply/patch:882: trailing whitespace. selects which template to use, either the opac or the intranet .git/rebase-apply/patch:2515: trailing whitespace. =cut .git/rebase-apply/patch:3763: trailing whitespace. =item cloud-kw.pl [--verbose|--help] --conf=F<cloud.conf> warning: 4 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/js/datatables.js CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/js/datatables.js Auto-merging Makefile.PL error: Failed to merge in the changes. Patch failed at 0001 Bug 39325: Automatic fix hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-39325-Automatic-fix-c49605tp.patch
Created attachment 181611 [details] [review] Bug 39325: Automatic fix Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181612 [details] [review] Bug 39325: Manual fixes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181613 [details] [review] Bug 39325: manual - reserv => reserve because of automatic changes in circ/transferstoreceive.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181614 [details] [review] Bug 39325: manual - hilighted => highlighted All the other occurrences have been updated. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181615 [details] [review] Bug 39325: manual - suggestion - no one => no_one It has been changed (automatically) from "noone" to "no one", which was wrong. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181616 [details] [review] Bug 39325: Adjust DB We need to adjust kohastructure.sql, sysprefs.sql and the DB itself. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181617 [details] [review] Bug 39325: Revert dbic schema files We need the md5sum to be adjusted. This should be done by the RM. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181618 [details] [review] Bug 39325: Tidy Some files need to be tidy after the automatic commit Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181619 [details] [review] Bug 39325: Exclude atomic update - TODO RM TODO: We will need to adjust this path with the db_rev one Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181620 [details] [review] Bug 39325: (follow-up) A few minor corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 181621 [details] [review] Bug 39325: (QA follow-up) Adjust some issues connexion is a product, added to ignore file tablee -> tables a few other small corrections Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
* Fixed file permission (+x) on codespell.t * Added a success message to the db update * Tidied fix-perl-path.PL * Moved database update * Adjusted path in Bug 39325: Exclude atomic update - TODO RM * Ran dbic after updatedatabase and comitted
Pushed for 25.05! Well done everyone, thank you!
Created attachment 181637 [details] [review] Bug 39325: (follow-up) Adjust DB
(In reply to Jonathan Druart from comment #77) > Created attachment 181637 [details] [review] [review] > Bug 39325: (follow-up) Adjust DB Patch pushed.
Created attachment 181651 [details] [review] Bug 39325: Fix t/Makefile.t
Follow-up pushed to main!
Created attachment 181983 [details] [review] Bug 39325: Skip codespell check for bullseye bullseye has 2.0.0-1 bookworm has 2.2.2-1 Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Follow-up "Skip codespell..." pushed to main (soon)