So far as I can tell, the last patch (for tools/inventory.pl) is the only one requiring sign-off.

Latest patch pushed

Resubmitted a patch from Jan. Turning on warnings causes a lot of messages due undefined branch and selected variables in members-home. This patch removes the cause of those "false positives"

Patch easily tested, doesn't change functionality just quiets warns. Pushed.

The following .pl and .pm files are still missing the use warnings directive:
C4/SIP/xmlparse.pl
acqui/check_budget_total.pl
acqui/fetch_sort_dropbox.pl
acqui/histsearch.pl
acqui/neworderbiblio.pl
acqui/newordersuggestion.pl
acqui/orderreceive.pl
acqui/parcel.pl
acqui/updatesupplier.pl
admin/aqbudget_owner_search.pl
admin/aqbudgetperiods.pl
admin/aqbudgets.pl
admin/aqplan.pl
admin/auth_subfields_structure.pl
admin/auth_tag_structure.pl
admin/categorie.pl
admin/check_budget_parent.pl
admin/check_parent_total.pl
admin/checkmarc.pl
admin/classsources.pl
admin/clone-rules.pl
admin/itemtypes.pl
admin/marc_subfields_structure.pl
admin/printers.pl
admin/smart-rules.pl
admin/z3950servers.pl
catalogue/ISBDdetail.pl
catalogue/MARCdetail.pl
catalogue/cardview.pl
catalogue/detailprint.pl
catalogue/export.pl
catalogue/issuehistory.pl
catalogue/moredetail.pl
catalogue/search.pl
catalogue/showmarc.pl
cataloguing/addbiblio.pl
cataloguing/additem.pl
cataloguing/merge.pl
cataloguing/merge_ajax.pl
cataloguing/moveitem.pl
cataloguing/plugin_launcher.pl
[all the files in cataloguing/value_builder]
cataloguing/ysearch.pl
cataloguing/z3950_search.pl
circ/bookcount.pl
circ/branchoverdues.pl
circ/circulation-home.pl
circ/circulation.pl
circ/hold-transfer-slip.pl
circ/pendingreserves.pl
circ/returns.pl
circ/stats.pl
circ/ysearch.pl
install-CPAN.pl
installer/data/mysql/atomicupdate/subscription_add_enddate.pl
installer/data/mysql/labels_upgrade.pl
installer/data/mysql/patroncards_upgrade.pl
installer/data/mysql/update22to30.pl
kohaversion.pl
members/deletemem.pl
members/guarantor_search.pl
members/member.pl
members/moremember.pl
members/notices.pl
members/update-child.pl
misc/batchCompareMARCvsFrameworks.pl
misc/batchDeleteUnusedSubfields.pl
misc/batchImportMARCWithBiblionumbers.pl
misc/batchRebuildBiblioTables.pl
misc/batchupdateISBNs.pl
misc/bin/zebraqueue_daemon.pl
misc/commit_biblios_file.pl
misc/cronjobs/build_browser_and_cloud.pl
misc/cronjobs/fines.pl
misc/cronjobs/holds/cancel_expired_holds.pl
misc/cronjobs/j2a.pl
misc/cronjobs/notifyMailsOp.pl
misc/cronjobs/smsoverdues.pl
misc/cronjobs/zebraqueue_start.pl
misc/exportauth.pl
misc/kohalib.pl
misc/migration_tools/22_to_30/export_Authorities.pl
misc/migration_tools/22_to_30/export_Authorities_xml.pl
misc/migration_tools/22_to_30/missing090field.pl
misc/migration_tools/22_to_30/move_marc_to_authheader.pl
misc/migration_tools/22_to_30/move_marc_to_biblioitems.pl
misc/migration_tools/22_to_30/rebuild_leader.pl
misc/migration_tools/22_to_30/rebuild_unimarc_100.pl
misc/migration_tools/build6xx.pl
misc/migration_tools/buildCOUNTRY.pl
misc/migration_tools/buildEDITORS.pl
misc/migration_tools/buildLANG.pl
misc/migration_tools/bulkauthimport.pl
misc/migration_tools/fix_onloan.pl
misc/migration_tools/merge_authority.pl
misc/migration_tools/rebuild_nozebra.pl
misc/migration_tools/rebuild_zebra.pl
misc/migration_tools/upgradeitems.pl
misc/stage_biblios_file.pl
misc/translator/stats.pl
misc/translator/text-extract.pl
misc/translator/text-extract2.pl
misc/translator/tmpl_process3.pl
opac/changelanguage.pl
opac/opac-privacy.pl
opac/opac-user.pl
reports/acquisitions_stats.pl
reports/bor_issues_top.pl
reports/borrowers_stats.pl
reports/cat_issues_top.pl
reports/catalogue_stats.pl
reports/guided_reports.pl
reports/issues_avg_stats.pl
reports/issues_stats.pl
reports/manager.pl
reports/reports-home.pl
reports/reservereport.pl
reports/reserves_stats.pl
reports/stats.print.pl
reports/stats.screen.pl
resetversion.pl
rotating_collections/addItems.pl
rotating_collections/editCollections.pl
rotating_collections/rotatingCollections.pl
rotating_collections/transferCollection.pl
sms/sms.pl
sms/sms_listen_windows_start.pl
suggestion/suggestion.pl
test/search.pl
tools/background-job-progress.pl
tools/batchMod.pl
tools/cleanborrowers.pl
tools/csv-profiles.pl
tools/koha-news.pl
tools/pdfViewer.pl
tools/picture-upload.pl
tools/scheduler.pl
tools/stage-marc-import.pl
tools/upload-file-progress.pl
tools/upload-file.pl
tools/viewlog.pl
virtualshelves/addbybiblionumber.pl

(In reply to comment #8)
> The following .pl and .pm files are still missing the use warnings directive:
> C4/SIP/xmlparse.pl
> acqui/check_budget_total.pl
> acqui/fetch_sort_dropbox.pl
...

heya Jared,
out of curiosity - whats the command or script used to generate this list?

All three attached files properly fix code for using warnings.  First patch adds 'my' to variables in foreach loops, and fixes some spacing.  Second patch corrects call to CGI (CGI->new(); instead of new CGI;).  Third patch breaks out default value of variable onto a second line with ||=.

Marking Passed QA.

Comment on attachment 9034 [details] [review]
Bug 2505 - enable Perl warnings in all modules and scripts

This patch has been pushed

Comment on attachment 9035 [details] [review]
Bug 2505 : use warnings in smart_rules.pl

This patch has been pushed

Comment on attachment 9036 [details] [review]
Bug 2505 : Switch on warnings in circ/returns.pl

This patch has been pushed

3 patches pushed, nothing waiting now, switching back to "assigned"

I've removed the use strict use warnings for C4/Ris.pm, because the script is not strict compliant and jenkins report a lot of problems:
Global symbol "$utf" requires explicit package name at C4/Ris.pm line 111.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 350.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 350.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 353.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 363.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 389.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 431.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 438.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 439.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 440.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 441.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 442.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 443.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 464.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 465.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 466.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 479.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 480.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 483.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 484.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 485.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 501.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 505.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 527.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 530.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 539.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 540.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 556.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 557.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 561.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 562.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 578.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 579.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 583.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 584.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 613.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 614.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 629.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 630.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 645.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 646.
Global symbol "$intype" requires explicit package name at C4/Ris.pm line 674.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 706.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 707.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 707.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 710.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 715.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 721.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 725.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 729.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 730.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 730.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 731.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 734.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 735.
Global symbol "$protoyear" requires explicit package name at C4/Ris.pm line 738.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 772.
Global symbol "@kwsubfields" requires explicit package name at C4/Ris.pm line 776.
Global symbol "@kwsubfields" requires explicit package name at C4/Ris.pm line 779.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 789.
Global symbol "@notesubfields" requires explicit package name at C4/Ris.pm line 892.
Global symbol "@notesubfields" requires explicit package name at C4/Ris.pm line 895.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 901.
Global symbol "$marcprint" requires explicit package name at C4/Ris.pm line 935.
Global symbol "$utf" requires explicit package name at C4/Ris.pm line 962.
Global symbol "$uniout" requires explicit package name at C4/Ris.pm line 966.
C4/Ris.pm had compilation errors.


(just commented the 2 lines, not reverted the whole patch = what is done is correct, but there are things missing)

(In reply to Andrew Maguire from comment #23)
> During testing, discovered the charconv() function is a no-op, so I wondered
> what
> should happen if database is in MARC8 (non-UTF) format.

Hi Andrew,

Can this happen ? unless I misunderstand something, Koha transform MARC8 into UTF when importing, so in the database, you should have only UTF records.

I agree with Paul - the database should never be in MARC8, if your database is not in UTF-8, you got another problem.

How can we continue here? The available patch does no longer apply.

Modern::Perl is used in C4::Ris since bug 12357.

This bug hasn't seen much attention for a few years now. During the Open Source Academy 2018 the students switched a lot (all?) scripts over to use Modern::Perl; instead of the separate use strict; use warnings; pragmas.

I think by not using use warnings; in those scripts, we have possibly been hiding problems and switching it on globally might now be the way to go. We can keep an eye on the logs as we are still rather early in the release cycle and keep fixing warns as we find them.

(In reply to Katrin Fischer from comment #27)
> This bug hasn't seen much attention for a few years now. During the Open
> Source Academy 2018 the students switched a lot (all?) scripts over to use
> Modern::Perl; instead of the separate use strict; use warnings; pragmas.

Nope, still few left.

Just to see what happens, I enabled warnings in C4/Reserves.pm. All of the warnings were of the form

Subroutine * redefined at C4/Reserves.pm line *.

This is happening because of circular dependencies, e.g. 


C4/Reserves.pm uses

C4::Members
C4::Circulation

Those modules in turn use C4::Reserves. None of the functions used in these modules is actually called in C4::Reserves, but removing the calls to C4::Members and C4::Circulation from C4/Reserves.pm isn't enough, apparently there's a deeper circular dependency there.

I suspect that there are a number of unnecessary 'use' statements peppered throughout the C4 namespace; cleaning those up might fix the problem entirely, or at least allow us to resolve any remaining dependencies easily.

Another Academy coming up... where else would we like students to make this change?

After bug 24217, this will no longer be a major.

Whats the current state of this one?

(In reply to Martin Renvoize from comment #32)
> Whats the current state of this one?

Still some left, but minor now:

% git grep -l 2505|grep -v '.md\|.html\|.txt'
cataloguing/value_builder/unimarc_field_105.pl
cataloguing/value_builder/unimarc_field_110.pl
cataloguing/value_builder/unimarc_field_120.pl
cataloguing/value_builder/unimarc_field_130.pl
cataloguing/value_builder/unimarc_field_140.pl
cataloguing/value_builder/unimarc_field_225a.pl
cataloguing/value_builder/unimarc_field_4XX.pl
koha-tmpl/intranet-tmpl/lib/linters/jshint.js
koha-tmpl/intranet-tmpl/lib/linters/jshint.min.js
koha-tmpl/opac-tmpl/lib/verovio/verovio-toolkit.js
misc/batchCompareMARCvsFrameworks.pl
misc/batchDeleteUnusedSubfields.pl
misc/batchImportMARCWithBiblionumbers.pl
misc/batchRebuildBiblioTables.pl
misc/cronjobs/build_browser_and_cloud.pl
misc/exportauth.pl
misc/kohalib.pl
misc/migration_tools/22_to_30/missing090field.pl
misc/migration_tools/22_to_30/move_marc_to_authheader.pl
misc/migration_tools/22_to_30/rebuild_leader.pl
misc/migration_tools/22_to_30/rebuild_unimarc_100.pl
misc/migration_tools/buildCOUNTRY.pl
misc/migration_tools/buildEDITORS.pl
misc/migration_tools/buildLANG.pl
misc/migration_tools/create_analytical_rel.pl
misc/migration_tools/upgradeitems.pl
misc/translator/tmpl_process3.pl
t/db_dependent/Biblio/TransformHtmlToMarc.t
t/db_dependent/data/marc21/zebraexport/biblio/exported_records

(should have excluded .js as well)