Summary: | Silly calculation of average time in touch_all scripts | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Command-line Utilities | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | david.bourgault, fridolin.somers, robin |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 19190: Silly calculation of average time in touch_all scripts
Bug 19190: [Follow-up] Do not rely on the return of ModItem Bug 19190: [Follow-up] Do not rely on the return of ModItem Bug 19190: Silly calculation of average time in touch_all scripts Bug 19190: [Follow-up] Do not rely on the return of ModItem Bug 19190: Silly calculation of average time in touch_all scripts Bug 19190: [Follow-up] Do not rely on the return of ModItem |
Description
Marcel de Rooy
2017-08-28 09:59:06 UTC
Created attachment 66550 [details] [review] Bug 19190: Silly calculation of average time in touch_all scripts When you want to calculate average time, do not divide count by time :) Test plan: Run the script with a where condition and verbose option and see that the average time is meaningful. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66551 [details] [review] Bug 19190: [Follow-up] Do not rely on the return of ModItem touch_all_items looks at the return of ModItem to determine if the operation was successful. But ModItem does not return a meaningful value. This patch puts the ModItem call in an eval and looks at $@. Test plan: Run touch_all_items with a where condition and verbose option. Put print 1/0; at the end of ModItem. Run touch_all_items again. You should see: ERROR WITH ITEM xxx !!!! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 67332 [details] [review] Bug 19190: [Follow-up] Do not rely on the return of ModItem touch_all_items looks at the return of ModItem to determine if the operation was successful. But ModItem does not return a meaningful value. This patch puts the ModItem call in an eval and looks at $@. Test plan: Run touch_all_items with a where condition and verbose option. Put print 1/0; at the end of ModItem. Run touch_all_items again. You should see: ERROR WITH ITEM xxx !!!! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Created attachment 67499 [details] [review] Bug 19190: Silly calculation of average time in touch_all scripts When you want to calculate average time, do not divide count by time :) Test plan: Run the script with a where condition and verbose option and see that the average time is meaningful. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Created attachment 67500 [details] [review] Bug 19190: [Follow-up] Do not rely on the return of ModItem touch_all_items looks at the return of ModItem to determine if the operation was successful. But ModItem does not return a meaningful value. This patch puts the ModItem call in an eval and looks at $@. Test plan: Run touch_all_items with a where condition and verbose option. Put print 1/0; at the end of ModItem. Run touch_all_items again. You should see: ERROR WITH ITEM xxx !!!! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> (In reply to David Bourgault from comment #3) > Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Thanks, David. I added your signoff line also to the first patch. Created attachment 67752 [details] [review] Bug 19190: Silly calculation of average time in touch_all scripts When you want to calculate average time, do not divide count by time :) Test plan: Run the script with a where condition and verbose option and see that the average time is meaningful. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 67753 [details] [review] Bug 19190: [Follow-up] Do not rely on the return of ModItem touch_all_items looks at the return of ModItem to determine if the operation was successful. But ModItem does not return a meaningful value. This patch puts the ModItem call in an eval and looks at $@. Test plan: Run touch_all_items with a where condition and verbose option. Put print 1/0; at the end of ModItem. Run touch_all_items again. You should see: ERROR WITH ITEM xxx !!!! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 17.11, thanks to everybody involved! Backport ? Pushed to 17.05.x, will be in 17.05.07 |