Description
Martin Renvoize (ashimema)
2018-10-19 13:31:22 UTC
Created attachment 80934 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Created attachment 80935 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> I cannot test this because of bug 11897 comment 366: % perl misc/cronjobs/stockrotation.pl --report=email --send-email --execute ERROR PROCESSING TEMPLATE: file error - parse error - input text line 5: unexpected token (ne) [% IF item.reason ne 'in-demand' %] at misc/cronjobs/stockrotation.pl line 343. Also note that the test plan is wrong, the correct value for --report is 'email', not 'mail' Created attachment 81055 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 81056 [details] [review] Bug 21620: (follow-up) Fix for bad TT syntax in default notice. Grr... silly template toolkit syntax error.. I think that followup should resolve it. (In reply to Martin Renvoize from comment #5) > Created attachment 81056 [details] [review] [review] > Bug 21620: (follow-up) Fix for bad TT syntax in default notice. Nope, you should fix other languages as well. Maybe doing it on a separate bug report would be better. Created attachment 81108 [details] [review] Bug 21620: (follow-up) Fix for bad TT syntax in default notice. Created attachment 81110 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 81111 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=email --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> As requested I've moved the followup patch to a second bug and committed the missing changes to it there. Created attachment 81872 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=email --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Awesome work all! Pushed to master for 18.11 Bug 21656 not in 18.05.x series. |