Description
Joy Nelson
2012-05-09 15:08:43 UTC
Created attachment 9489 [details] [review] Bug 8063 - Hold print notices do not sort by branch Adds the option -s/--split to enable notices to be separated into different files by borrower home library. Created attachment 9514 [details] [review] [SIGNED-OFF] Bug 8063 - Hold print notices do not sort by branch Works as expected. Signed off. QA comments: * a lot of code cleaning that make the visibility of the real changes hard to spot ! * perlcritic is failing. Before the patch there is one error, after the patch there are 2: Bareword file handle opened at line 82, column 9. See pages 202,204 of PBP. (Severity: 5) Bareword file handle opened at line 107, column 5. See pages 202,204 of PBP. (Severity: 5) * tiny SQL comment: + LEFT JOIN borrowers b on b.borrowernumber = mq.borrowernumber should be written with UC for on: + LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber (could also be written + LEFT JOIN borrowers b USING(borrowernumber) ) Please fix both (they are the same error) and resubmit. If you submit a follow-up, directly switch to signed-off Created attachment 9696 [details] [review] Bug 8063 - Hold print notices do not sort by branch Adds the option -s/--split to enable notices to be separated into different files by borrower home library. Created attachment 9697 [details] [review] Bug 8063 - Followup - Make C4/Letters.pm conform to PBP Perlcritic reported the following errors: Subroutine prototypes used at line 96, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 120, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 152, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 173, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 190, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 227, column 1. See page 194 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 228, column 31. See page 199 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 229, column 31. See page 199 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 236, column 9. See page 199 of PBP. (Severity: 5) Subroutine prototypes used at line 668, column 1. See page 194 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 669, column 27. See page 199 of PBP. (Severity: 5) Subroutine prototypes used at line 719, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 865, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 899, column 1. See page 194 of PBP. (Severity: 5) Subroutine prototypes used at line 981, column 1. See page 194 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 982, column 28. See page 199 of PBP. (Severity: 5) Subroutine prototypes used at line 1000, column 1. See page 194 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 1001, column 27. See page 199 of PBP. (Severity: 5) "return" statement with explicit "undef" at line 1004, column 9. See page 199 of PBP. (Severity: 5) I do apologize for the code cleanup blocking the visibility of changes. I figured this was a good file to tidy as it is rarely modified so the likelihood of the cleaning breaking another patch is extremely low. If I continue this practice, I will submit a the cleanup as a separate followup patch. I had not know that we should be updating code to conform to PBP, but it's a very good idea. In that spirit, I have submitted a followup to conform C4/Letters.pm to PBP. Created attachment 9912 [details] [review] [SIGNED-OFF] Bug 8063 - Hold print notices do not sort by branch Still works ! Created attachment 9913 [details] [review] [SIGNED-OFF] Bug 8063 - Followup - Make C4/Letters.pm conform to PBP I don't understand why making things explicit is a high severity violation of best practices... but the patch is ok and perlcritic doesn't complain anymore. First patch: Moves some code around, as well as adding a new loop and hash for splitting by branchcode. Marking Passed QA. Second patch: Just PBP cleanup, safe and sane, passed QA. mmm... Kyle/Julian, When trying to run this script I gat a nasty: 15:42 ~/koha.dev/koha-community (new/bug_8063 $%)$ ./misc/cronjobs/gather_print_notices.pl Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 81. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 95. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 102. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 106. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 120. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 127. Execution of ./misc/cronjobs/gather_print_notices.pl aborted due to compilation errors. WTF ? changing severity, it's an ENH, as well as assignee Created attachment 9953 [details] [review] Bug 8063 - Followup - Bug fix $OUTPUT being used but not being declared. When trying to run this script I gat a nasty: 15:42 ~/koha.dev/koha-community (new/bug_8063 $%)$ ./misc/cronjobs/gather_print_notices.pl Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 81. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 95. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 102. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 106. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 120. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 127. Execution of ./misc/cronjobs/gather_print_notices.pl aborted due to compilation errors. Created attachment 10786 [details] [review] Bug 8063 - [SIGNED-OFF] Followup - Bug fix $OUTPUT being used but not being declared. When trying to run this script I gat a nasty: 15:42 ~/koha.dev/koha-community (new/bug_8063 $%)$ ./misc/cronjobs/gather_print_notices.pl Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 81. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 95. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 102. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 106. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 120. Global symbol "$OUTPUT" requires explicit package name at ./misc/cronjobs/gather_print_notices.pl line 127. Execution of ./misc/cronjobs/gather_print_notices.pl aborted due to compilation errors. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Before the patch: $perl -wc ./misc/cronjobs/gather_print_notices.pl Global symbol "$OUTPUT" requires explicit package name at [...]./misc/cronjobs/gather_print_notices.pl line 81. ./misc/cronjobs/gather_print_notices.pl had compilation errors. With this patch: $perl -wc ./misc/cronjobs/gather_print_notices.pl ./misc/cronjobs/gather_print_notices.pl syntax OK Only the last patch is waiting for QA QA comment: No more comments, passed QA ! Tested before pushing: * UPDATE message_queue SET status='pending' WHERE status=sent AND message_transport_type='print'; to set some message to pending status * run misc/cronjobs/gather_print_notices.pl tmp => VERIFIED that the print is in tmp/holdnotices-2012-07-16.html, and contains notices * UPDATE message_queue SET status='pending' WHERE status=sent AND message_transport_type='print'; to set some message to pending status * run misc/cronjobs/gather_print_notices.pl -s tmp => VERIFIED that the print is in tmp/holdnotices-2012-07-16-MAURES.html (MAURES being my branchcode), and contains notices |