[Bug 17447] New: Housekeeping: Tidy up misc/batchRebuildItemsTables.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Bug ID: 17447 Summary: Housekeeping: Tidy up misc/batchRebuildItemsTables.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: trivial Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 56516 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56516&action=edit Bug 17447: Housekeeping: Tidy up misc/batchRebuildItemsTables.pl This script contained lots of complete if blocks on one line. In this patch a lot of whitespace has been moved around. A line with a call to _build_tag_directory has been removed; the results were discarded. Variable $extkey was not used either. The script now does not only print help if you specify -t. Test plan: Run the script. Do not test on production data! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56516|0 |1 is obsolete| | --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 56578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56578&action=edit Bug 17447: Housekeeping: Tidy up misc/batchRebuildItemsTables.pl This script contained lots of complete if blocks on one line. In this patch a lot of whitespace has been moved around. A line with a call to _build_tag_directory has been removed; the results were discarded. Variable $extkey was not used either. The script now does not only print help if you specify -t. Test plan: Run the script. Do not test on production data! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 56578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56578 Bug 17447: Housekeeping: Tidy up misc/batchRebuildItemsTables.pl Review of attachment 56578: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17447&attachment=56578) ----------------------------------------------------------------- ::: misc/batchRebuildItemsTables.pl @@ +35,3 @@
my $starttime = gettimeofday; my @errors; my %opt;
Is this even used? @@ +43,4 @@
'where:s' => \$where, ) or pod2usage(2);
+pod2usage(1) if $help || ( !$confirm && !$test_parameter );
Tweaks and whitespace changes should be separate commits to make it easier to check. git diff -w origin/master pointed out this and a couple others. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. Tompsett from comment #3)
my $starttime = gettimeofday; my @errors; my %opt;
Is this even used? The first two are used, the third is not. Removing it now too.
Tweaks and whitespace changes should be separate commits to make it easier to check. So true :) Added a perltidy patch.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56578|0 |1 is obsolete| | --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58523 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58523&action=edit Bug 17447: Run perltidy on batchRebuildItemsTables.pl Test plan: Verify that the output of diff -w between the original and tidied file does not introduce code changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58524 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58524&action=edit Bug 17447: Remove unused vars from batchRebuildItemsTables.pl Variables $extkey not used, %opt not used. Variables $tmptest[...] not used, calling _build_tag_directory useless. The script now does not only print help if you specify -t. Sub defnonull tidied. Rearranged modules, removed Dumper. Test plan: [1] Run the script with -t flag. The script should not only print usage statement, but should do a dry run. (Test this on a small database, or pass an additional where clause.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Housekeeping: Tidy up |Remove unused vars from |misc/batchRebuildItemsTable |batchRebuildItemsTables.pl |s.pl | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58528&action=edit Bug 17447: %opt is unused Deleted the line. perlcritic -4 before and after. patch will have one less difference. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 58528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58528 Bug 17447: %opt is unused Review of attachment 58528: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17447&attachment=58528) ----------------------------------------------------------------- ::: misc/batchRebuildItemsTables.pl @@ +109,4 @@
sub defnonull { my $var = shift; + return defined $var && $var ne q{};
This was so I could get perlcritic -4. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58523|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58529 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58529&action=edit Bug 17447: Run perltidy on batchRebuildItemsTables.pl Test plan: Verify that the output of diff -w between the original and tidied file does not introduce code changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58524|0 |1 is obsolete| | --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58530&action=edit Bug 17447: Remove unused vars from batchRebuildItemsTables.pl Variables $extkey not used, %opt not used. Variables $tmptest[...] not used, calling _build_tag_directory useless. The script now does not only print help if you specify -t. Sub defnonull tidied. Rearranged modules, removed Dumper. Test plan: [1] Run the script with -t flag. The script should not only print usage statement, but should do a dry run. (Test this on a small database, or pass an additional where clause.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> NOTE: Fixed %opt in my followup. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58529|0 |1 is obsolete| | --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58531 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58531&action=edit Bug 17447: Run perltidy on batchRebuildItemsTables.pl Test plan: Verify that the output of diff -w between the original and tidied file does not introduce code changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58530|0 |1 is obsolete| | --- Comment #12 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58532 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58532&action=edit Bug 17447: Remove unused vars from batchRebuildItemsTables.pl Variables $extkey not used, %opt not used. Variables $tmptest[...] not used, calling _build_tag_directory useless. The script now does not only print help if you specify -t. Sub defnonull tidied. Rearranged modules, removed Dumper. Test plan: [1] Run the script with -t flag. The script should not only print usage statement, but should do a dry run. (Test this on a small database, or pass an additional where clause.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58528|0 |1 is obsolete| | --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 58533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58533&action=edit Bug 17447: %opt is unused Deleted the line. perlcritic -4 before and after. Before there are issues. After there is not. Also, changed function to not rely on implicit return value of last line, but explicitly stated a return. And operator changed, due to precedence issues. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. Tompsett from comment #13)
Created attachment 58533 [details] [review] Bug 17447: %opt is unused
Deleted the line. perlcritic -4 before and after. Before there are issues. After there is not.
Also, changed function to not rely on implicit return value of last line, but explicitly stated a return. And operator changed, due to precedence issues.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Thx Mark. I am sure to have deleted the opt line, but pretty sure now too that i did not commit that last change ;) The change in defnonull is fine with me, but the QAer may also remove it completely. We could just say something like pref(merge) || 0. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58533|0 |1 is obsolete| | --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58534 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58534&action=edit Bug 17447: %opt is unused Deleted the line. perlcritic -4 before and after. Before there are issues. After there is not. Also, changed function to not rely on implicit return value of last line, but explicitly stated a return. And operator changed, due to precedence issues. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58531|0 |1 is obsolete| | Attachment #58532|0 |1 is obsolete| | Attachment #58534|0 |1 is obsolete| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 58537 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58537&action=edit Bug 17447: Run perltidy on batchRebuildItemsTables.pl Test plan: Verify that the output of diff -w between the original and tidied file does not introduce code changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 58538 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58538&action=edit Bug 17447: Remove unused vars from batchRebuildItemsTables.pl Variables $extkey not used, %opt not used. Variables $tmptest[...] not used, calling _build_tag_directory useless. The script now does not only print help if you specify -t. Sub defnonull tidied. Rearranged modules, removed Dumper. Test plan: [1] Run the script with -t flag. The script should not only print usage statement, but should do a dry run. (Test this on a small database, or pass an additional where clause.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 58539 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58539&action=edit Bug 17447: %opt is unused Deleted the line. perlcritic -4 before and after. Before there are issues. After there is not. Also, changed function to not rely on implicit return value of last line, but explicitly stated a return. And operator changed, due to precedence issues. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Marcel, Mark! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Master |RESOLVED CC| |katrin.fischer@bsz-bw.de Severity|trivial |enhancement --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This looks like code clean-up - treating this as an enhancement. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #21 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 58538 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58538 Bug 17447: Remove unused vars from batchRebuildItemsTables.pl Review of attachment 58538: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17447&attachment=58538) ----------------------------------------------------------------- ::: misc/batchRebuildItemsTables.pl @@ +43,4 @@
'where:s' => \$where, ) or pod2usage(2);
+pod2usage(1) if $help || ( !$confirm && !$test_parameter );
This is the only reason you would consider it an enhancement, but it behaves more like a bug fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17447 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Which bug does it fix? If there is a test plan to verify it fixes an existing bug, I might reconsider. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org