Description
Marc Véron
2015-09-22 14:00:36 UTC
With all the blocking Bugs pushed (latest is Bug 14998), the only remaining files should be: To be removed: C4/Dates.pm t/Dates.t To be changed: debian/templates/plack.psgi misc/plack/koha.psgi 17 patches left to sign-off as of today. 13 patches left to sign-off as of today As of today - 7 patches left to sign-off - 1 signed off As of today depending on: Need Sign-off: 14954 14946 14960 14969 Signed-off 14985 14965 14998 14956 I switch to "Needs sign-off" Created attachment 44595 [details] [review] Bug 14870 - Remove C4/Dates.pm from Koha Remove C4/Dates.pm form Koha Hi Marc I attached the patch for the near future. Regards Hi Héctor, Thanks for the patch for removing C4/Dates.pm With all the patches applied, we can finally use the new date format from Bug 12072 However, before we remove C4/Dates.pm we have to make sure that the file really no longer is used or referenced. See my comment #1: > To be removed: > > C4/Dates.pm > t/Dates.t > > To be changed: > > debian/templates/plack.psgi > misc/plack/koha.psgi Additionally, we should do a final global search for C4::Dates. As of today, with following patches applied: 14960 (nso) 14969 (nqa) 14985 (nqa) 14954 (nqa) 14946 (nqa) 14998 (nqa) Rusults from: git grep 'C4::Dates': Form comment #1: C4/Dates.pm (remove) t/Dates.t (remove) debian/templates/plack.psgi (change to remove C4::Dates) misc/plack/koha.psgi (change to remove C4::Dates) Some stray C4::Dates found in: C4/Debug.pm (follow up) C4/Overdues.pm (follow up) C4/Serials.pm (follow up) circ/returns.pl (follow up) reports/borrowers_stats.pl (follow up) reports/guided_reports.pl (follow up) t/DateUtils.t (OK, it's in a comment only) misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl (remove? follow up?) Created attachment 44612 [details] [review] Bug 14870 - Remove t/Dates.t from Koha Hi Marc. Please help me out with koha.psgi and plack.psgi I don't want to screw up. (In reply to Héctor Eduardo Castro Avalos from comment #11) > Hi Marc. > Please help me out with koha.psgi and plack.psgi I don't want to screw up. Just remove it. It is only used for preloading it. Created attachment 44613 [details] [review] Bug 14870 - Remove C4::Dates from plack.psgi and koha.psgi Remove C4::Dates from .psgi files from preloaded modules, placing Koha::DateUtils instead (In reply to Tomás Cohen Arazi from comment #12) > Just remove it. It is only used for preloading it. Done. But I place instead Koha::DateUtils, correct me if I'm wrong. (In reply to Marc Véron from comment #9) (...) > Some stray C4::Dates found in: (...) > C4/Serials.pm (follow up) (...) Fixed in follow up to Bug 14969 I will fix some remaining stray C4::Dates in follow-ups and/or a separate bug. Some stray C4::Dates found in: C4/Debug.pm (follow up) C4/Overdues.pm (follow up) circ/returns.pl (follow up) reports/borrowers_stats.pl (follow up) reports/guided_reports.pl (follow up) t/DateUtils.t (OK, it's in a comment only) misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl (remove? follow up?) Switching to "Needs sign-off" C4/Overdues.pm fixed in followup in Bug 14985 Created attachment 44623 [details] [review] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl This patch removes 1 remaining / stray occurence of C4::Dates from circ/returns.pl To test: - Apply patch - Go to Home > Circulation > Check in - Verfiy that check-ins with overriding return dates work as before. Created attachment 44625 [details] [review] Bug 14870: (followup) Remove stray C4::Dates from reports files This patch removes stray C4::Dates from following files: reports/borrowers_stats.pl To test: Verify that C4::Dates is not used in the file reports/guided_reports.pl: To test: - Run a saved report with date input and with date range input, e.g. - Verify that you are asked for date range or date and that the results are the same as without patch Created attachment 44626 [details] [review] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm C4::Dates is mentioned as example in C4/Debug.pm. This patch removes it. Still applies. It appears a few are missing. I tested this on top of: 14954 14946 I found those references outside the release notes: C4/Serials.pm:$planneddate is a C4::Dates object. misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl:use C4::Dates; serials/acqui-search-result.pl:use C4::Dates qw/format_date/; t/DateUtils.t:# C4::Dates allowed 00th of the month 2 of those appear to be comments - but please check. Looked a bit further: 1) C4/Serials.pm:$planneddate is a C4::Dates object. - POD, should be corrected 2) misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl:use C4::Dates; - Covered by bug 15076. We need to make a decision there. 3) serials/acqui-search-result.pl:use C4::Dates qw/format_date/; - This one needs to be fixed, has a few date_format. 4) t/DateUtils.t:# C4::Dates allowed 00th of the month - I think that can probably be ignored Looking for format_date I find one more ocurrence: 5) C4/Log.pm: $data->{day} = format_date($data->{timestamp}); - Looks like this was missed. format_date_in_iso: 6) C4/Items.pm: $datelastseen = format_date_in_iso($datelastseen); 7) t/db_dependent/Items/GetItemsForInventory.t: $datelastseen = format_date_in_iso($datelastseen); 8) tools/koha-news.pl: #$new->{'expirationdate'}=format_date_in_iso($new->{'expirationdate'}); Created attachment 44943 [details] [review] Bug 14969: (followup) Remove stray C4::Dates (See comment #23) This patch removes stray C4::Dates (date_fromat...) from C4/Items.pm C4/Log.pm C4/Serials.pm serials/acqui-search-result.pl t/DateUtils.t t/db_dependent/Items/GetItemsForInventory.t tools/koha-news.pl Some of them were inside comments etc. To test: - git grep 'C4::Dates' should give no result - git grep 'format_d' should give no result Exception: in one cron job there exists an own sub format_date, and occurences not related to C4::Dates - Search for regressions Created attachment 44944 [details] [review] Bug 14870: (followup) Remove stray C4::Dates (See comment #23) This patch removes stray C4::Dates (date_fromat...) from C4/Items.pm C4/Log.pm C4/Serials.pm serials/acqui-search-result.pl t/DateUtils.t t/db_dependent/Items/GetItemsForInventory.t tools/koha-news.pl Some of them were inside comments etc. To test: - git grep 'C4::Dates' should give no result - git grep 'format_d' should give no result Exception: in one cron job there exists an own sub format_date, and occurences not related to C4::Dates - Search for regressions Hi, I found that in reports/guided_reports.pl C4:Dates is still present: 359 # If the range values are dates 360 if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~ C4::Dates->regexp('syspref')) { 361 $fromvalue = C4::Dates->new($fromvalue)->output("iso"); 362 $tovalue = C4::Dates->new($tovalue)->output("iso"); 363 } Hi, I found that in reports/guided_reports.pl C4:Dates is still present. From lines 359 to 363: # If the range values are dates if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~ C4::Dates->regexp('syspref')) { $fromvalue = C4::Dates->new($fromvalue)->output("iso"); $tovalue = C4::Dates->new($tovalue)->output("iso"); } A new bug or you fix here ? Hi Zeno, thakns for testing. I can not find C4::Dates in reports/guided_reports.pl Git grep 'C4::Dates' only gives results only in misc/release_notes/release_notes_3_22_0.txt I applyed the patches after a recent git pull: HEAD is now at 87e7d55 Bug 15199: t/db_dependent/Review.t should not depend on existing data Switching back to NSO Created attachment 44954 [details] [review] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl This patch removes C4::Dates from fix_accountlines_rmdupfines_bug8253.pl To test: - Apply patch - Check code for uses of C4::Dates (date_format) (In reply to Zeno Tajoli from comment #27) > Hi, I found that in reports/guided_reports.pl C4:Dates is still present. > > From lines 359 to 363: > > # If the range values are dates > if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~ > C4::Dates->regexp('syspref')) { > $fromvalue = C4::Dates->new($fromvalue)->output("iso"); > $tovalue = C4::Dates->new($tovalue)->output("iso"); > } > > A new bug or you fix here ? A previous attachment in this bug fixed it up <http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44625&action=diff> Created attachment 44997 [details] [review] [SIGNED OFF] remove C4/Dates.pm from Koha http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 44998 [details] [review] [SIGNED OFF] remove C4/Dates.pm from Koha http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 44999 [details] [review] [SIGNED OFF] Bug 14870 - Remove t/Dates.t from Koha Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45000 [details] [review] [SIGNED OFF] Remove C4::Dates from plack.psgi and koha.psgi Remove C4::Dates from .psgi files from preloaded modules, placing Koha::DateUtils instead http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45001 [details] [review] [SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl This patch removes 1 remaining / stray occurence of C4::Dates from circ/returns.pl To test: - Apply patch - Go to Home > Circulation > Check in - Verfiy that check-ins with overriding return dates work as before. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45002 [details] [review] [SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates from reports files This patch removes stray C4::Dates from following files: reports/borrowers_stats.pl To test: Verify that C4::Dates is not used in the file reports/guided_reports.pl: To test: - Run a saved report with date input and with date range input, e.g. - Verify that you are asked for date range or date and that the results are the same as without patch Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45003 [details] [review] [SIGNED OFF] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm C4::Dates is mentioned as example in C4/Debug.pm. This patch removes it. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45004 [details] [review] [SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates (See comment #23) This patch removes stray C4::Dates (date_fromat...) from C4/Items.pm C4/Log.pm C4/Serials.pm serials/acqui-search-result.pl t/DateUtils.t t/db_dependent/Items/GetItemsForInventory.t tools/koha-news.pl Some of them were inside comments etc. To test: - git grep 'C4::Dates' should give no result - git grep 'format_d' should give no result Exception: in one cron job there exists an own sub format_date, and occurences not related to C4::Dates - Search for regressions http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> serials/acqui-search-results.pl looks like it should be revisited, containing code that might not be needed. Searching a vendor in serials still works witout a problem. Created attachment 45005 [details] [review] [PASSED QA] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl This patch removes C4::Dates from fix_accountlines_rmdupfines_bug8253.pl To test: - Apply patch - Check code for uses of C4::Dates (date_format) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 45006 [details] [review] Bug 14870: QA follow-up - fixing tiny typo Created attachment 45007 [details] [review] [PASSED QA] Bug 14870: Remove C4/Dates.pm from Koha http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45008 [details] [review] [PASSED QA] Bug 14870: Remove t/Dates.t from Koha Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45009 [details] [review] [PASSED QA] Bug 14870: Remove C4::Dates from plack.psgi and koha.psgi Remove C4::Dates from .psgi files from preloaded modules, placing Koha::DateUtils instead http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45010 [details] [review] [PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl This patch removes 1 remaining / stray occurence of C4::Dates from circ/returns.pl To test: - Apply patch - Go to Home > Circulation > Check in - Verfiy that check-ins with overriding return dates work as before. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45011 [details] [review] [PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from reports files This patch removes stray C4::Dates from following files: reports/borrowers_stats.pl To test: Verify that C4::Dates is not used in the file reports/guided_reports.pl: To test: - Run a saved report with date input and with date range input, e.g. - Verify that you are asked for date range or date and that the results are the same as without patch Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45012 [details] [review] [PASSED QA] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm C4::Dates is mentioned as example in C4/Debug.pm. This patch removes it. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45013 [details] [review] [PASSED QA] Bug 14870: (followup) Remove stray C4::Dates (See comment #23) This patch removes stray C4::Dates (date_fromat...) from C4/Items.pm C4/Log.pm C4/Serials.pm serials/acqui-search-result.pl t/DateUtils.t t/db_dependent/Items/GetItemsForInventory.t tools/koha-news.pl Some of them were inside comments etc. To test: - git grep 'C4::Dates' should give no result - git grep 'format_d' should give no result Exception: in one cron job there exists an own sub format_date, and occurences not related to C4::Dates - Search for regressions http://bugs.koha-community.org/show_bug.cgi?id=14870 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> serials/acqui-search-results.pl looks like it should be revisited, containing code that might not be needed. Searching a vendor in serials still works witout a problem. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45014 [details] [review] [PASSED QA] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl This patch removes C4::Dates from fix_accountlines_rmdupfines_bug8253.pl To test: - Apply patch - Check code for uses of C4::Dates (date_format) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 45015 [details] [review] [PASSED QA] Bug 14870: QA follow-up - fixing tiny typo Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> In the words of cait: Die C4::Dates, die! Code all checks out, tests all pass and my testing also showed nothing untoward.. passing! Patches pushed to master. Thanks Hector and Marc! |