Summary: | output_pref should use a hashref for parameters | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Circulation | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | gmcharlt, kyle.m.hall, srdjan |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 10380: Change prototype for output_pref routine
[SIGNED-OFF] Bug 10380: Change prototype for output_pref routine Bug 10380: Change prototype for output_pref routine Bug 10380: Followup Change prototype for output_pref routine |
Description
Jonathan Druart
2013-05-30 12:37:46 UTC
Created attachment 18517 [details] [review] Bug 10380: Change prototype for output_pref routine Koha::DateUtils::output_pref takes 4 parameters and the last one is a boolean, so some calls are: output_pref($dt, undef, undef, 1) This patch changes it prototype to output_pref({ dt => $dt, dateformat => $dateformat, timeformat => $timeformat, dateonly => $boolean }); an alternative is to call the output_pref routine with a datetime object, without using an hashref: output_pref($dt); Test plan: Go on some page where dates are displayed (circulation, laterorder, member page, etc.). Launch the unit tests file t/DateUtils.t. Created attachment 18597 [details] [review] [SIGNED-OFF] Bug 10380: Change prototype for output_pref routine Koha::DateUtils::output_pref takes 4 parameters and the last one is a boolean, so some calls are: output_pref($dt, undef, undef, 1) This patch changes it prototype to output_pref({ dt => $dt, dateformat => $dateformat, timeformat => $timeformat, dateonly => $boolean }); an alternative is to call the output_pref routine with a datetime object, without using an hashref: output_pref($dt); Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Created attachment 19188 [details] [review] Bug 10380: Change prototype for output_pref routine Koha::DateUtils::output_pref takes 4 parameters and the last one is a boolean, so some calls are: output_pref($dt, undef, undef, 1) This patch changes it prototype to output_pref({ dt => $dt, dateformat => $dateformat, timeformat => $timeformat, dateonly => $boolean }); an alternative is to call the output_pref routine with a datetime object, without using an hashref: output_pref($dt); Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to comment #1) > This patch changes it prototype to > output_pref({ > dt => $dt, If we switch to named parameters, we may as well avoid over-abbreviating. Would there be any objection to a followup that renames 'dt' to 'date'? Setting to "In discussion" pending the patch submitter answering the question I posed in comment 4. (In reply to Galen Charlton from comment #5) > Setting to "In discussion" pending the patch submitter answering the > question I posed in comment 4. Oops sorry Galen, I did not see it! dt is an abbreviation for datetime, not date. So I don't think "date" is an appropriate name. However we could choose "datetime" instead. Galen, should I switch this report to 'Passed QA'? Created attachment 20906 [details] [review] Bug 10380: Followup Change prototype for output_pref routine Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Pushed to master, along with follow-ups to update a couple new calls to output_pref() and to improve the POD. Thanks, Jonathan! |