| Summary: | more options for serialsUpdate.pl | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Fridolin Somers <fridolin.somers> |
| Component: | Command-line Utilities | Assignee: | Fridolin Somers <fridolin.somers> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | bgkriegel, gmcharlt, mathsabypro |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Proposed patch
Proposed patch (followup) Proposed patch Proposed patch (followup) [SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl [SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl (followup) Bug 9739 - followup - adds missing comma [PASSED QA] Bug 9739: more options for serialsUpdate.pl [PASSED QA] Bug 9739: more options for serialsUpdate.pl (followup) [PASSED QA] Bug 9739 - followup - adds missing comma Bug 9739: Follow-up - Improve help text a bit |
||
|
Description
Fridolin Somers
2013-03-01 13:16:05 UTC
Created attachment 15800 [details] [review] Proposed patch See commit message. Created attachment 15801 [details] [review] Proposed patch (followup) Follow up patch for cosmetic changes : - perltidy - q{} for SQL query instead of "" - list of columns is SQL query instead of SELECT * Fridolyn, I suppose the 1st attachment is obsolete? M. Saby (In reply to comment #3) > Fridolyn, I suppose the 1st attachment is obsolete? No, 1st attachement is the main patch. Your followup looks strange : it contains the same strings as the main patch. Mathieu Created attachment 18058 [details] [review] Proposed patch Created attachment 18059 [details] [review] Proposed patch (followup) (In reply to comment #5) > Your followup looks strange : it contains the same strings as the main patch. > > Mathieu Indeed. I recreated patches form my branch and reattached them. Thanks. Created attachment 20871 [details] [review] [SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl Script serialsUpdate.pl must be set in crontab to run daily. It checks serials that are late, modifies status and adds a note. This patch adds some options : --note : defined note, a defaut one is used if not defined --no-note : disable automatic note --verbose : used to control output This patch also implements confirmation option "-c" who whas present but not used. Test plan : Run script on a database with late serials : - serialsUpdate.pl -h => you get help text in output - serialsUpdate.pl --man => you get full help - serialsUpdate.pl -v => you get output of changed serials but database has not changed - serialsUpdate.pl -c -v => you get output of changed serials and database has changed, with note "Automatically set to late" - serialsUpdate.pl -c => database has changed without anything in output - serialsUpdate.pl -c --note "LATE" => database has changed, with note "LATE" - serialsUpdate.pl -c --no-note => database has changed with no note Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described. No koha-qa errors. Created attachment 20872 [details] [review] [SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl (followup) cosmetic changes : perltidy q{} for query list of columns is query instead of SELECT * Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Your cosmetic changes missed a comma, fixed in a followup Created attachment 20873 [details] [review] Bug 9739 - followup - adds missing comma Adds a missing comma in select statement. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> I have found some smaller problems with this script that I wanted to note: 1) When I start the script without giving any parameters nothing happens. Maybe it would be helpful to print out the help in this case? 2) The script updates serial issues that are already late and claimed with the note. I think it shouldn't touch issues that are already marked late in the interface. 3) The script overwrites and deletes existing notes on the issues. I think all this problems predate your patch, but I am quite worried about 2) and 3). The existing notes can now be preserved using the --no-note option, but I think ideally the new notes should be amended to the existing one in general. Still, this patch is an improvement, so it will pass. Created attachment 21588 [details] [review] [PASSED QA] Bug 9739: more options for serialsUpdate.pl Script serialsUpdate.pl must be set in crontab to run daily. It checks serials that are late, modifies status and adds a note. This patch adds some options : --note : defined note, a defaut one is used if not defined --no-note : disable automatic note --verbose : used to control output This patch also implements confirmation option "-c" who whas present but not used. Test plan : Run script on a database with late serials : - serialsUpdate.pl -h => you get help text in output - serialsUpdate.pl --man => you get full help - serialsUpdate.pl -v => you get output of changed serials but database has not changed - serialsUpdate.pl -c -v => you get output of changed serials and database has changed, with note "Automatically set to late" - serialsUpdate.pl -c => database has changed without anything in output - serialsUpdate.pl -c --note "LATE" => database has changed, with note "LATE" - serialsUpdate.pl -c --no-note => database has changed with no note Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described. No koha-qa errors. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. New options behave as described. There are some existing problems with this script that I have noted directly on the bug report. Created attachment 21589 [details] [review] [PASSED QA] Bug 9739: more options for serialsUpdate.pl (followup) cosmetic changes : perltidy q{} for query list of columns is query instead of SELECT * Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Your cosmetic changes missed a comma, fixed in a followup Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 21590 [details] [review] [PASSED QA] Bug 9739 - followup - adds missing comma Adds a missing comma in select statement. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 21592 [details] [review] Bug 9739: Follow-up - Improve help text a bit Changes only strings in the script: the help text and the success message for updated serial issues. Thx to Jared for proof reading! Pushed to master. Thanks, Fridolyn! |