Description
Colin Campbell
2010-11-29 10:13:09 UTC
I realize Conway says that postfixed if's are not good style, but I think that as this is a coding "style" issue, it should at least be given an airing on the devel list prior to being implemented. The issue isn't the postfix if per se but the declaration of the variable within a the conditional. It can cause run time warnings when the undefined undeclined variable is used later in the code This bug is mentioned in: Bug 5453 Do not declare variables in conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013116.html Bug 5453: Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013120.html Bug 5453: Move declarations from within conditionals in catalogue/search.pl http://lists.koha-community.org/pipermail/koha-patches/2010-November/013121.html Bug 5453 Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013122.html Bug 5453 Move declarations out of conditionals in opac http://lists.koha-community.org/pipermail/koha-patches/2010-November/013123.html Bug 5453 Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013124.html Bug 5453 : Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013125.html Could you attach the proposed patches? Created attachment 4340 [details] [review] Proposed Patch Created attachment 4341 [details] [review] Proposed Patch Created attachment 4342 [details] [review] Proposed Patch Created attachment 4343 [details] [review] Proposed Patch Created attachment 4344 [details] [review] Proposed Patch Created attachment 4345 [details] [review] Proposed Patch Created attachment 4346 [details] [review] Proposed Patch Patches rebased against current HEAD and attached or they can be pulled from the bug_5453 branch on git://github.com/colinsc/koha.git Created attachment 4455 [details] [review] Bug 5453 Do not declare variables in conditionals Removed occurences in acqui/*.pl Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4456 [details] [review] Bug 5453: Move declarations out of conditionals Patch admin/*.pl scripts Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4457 [details] [review] Bug 5453: Move declarations from within conditionals in catalogue/search.pl Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4458 [details] [review] Bug 5453 Move declarations out of conditionals Fix im members scripts Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4459 [details] [review] Bug 5453 Move declarations out of conditionals in opac Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4460 [details] [review] Bug 5453 Move declarations out of conditionals Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 4461 [details] [review] Bug 5453 : Move declarations out of conditionals Patched for C4 Modules Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Thanks for doing this Colin, it quiets the logs a lot! 1st attachment: acqui/*.pl: Passed QA 2nd attachment: admin/*.pl: Passed QA 3rd attachment: catalogue/search.pl: Passed QA 4th attachment: members/*.pl: Passed QA 5th attachment: opac/*.pl: Passed QA 6th attachment: lots of others: Passed QA 7th attachment: will require more time to read through, as the changes seem more complex, and will affect more of the system. Please stand by. 7th attachment: C4 modules: Passed QA. Marking bug report as such. Pushed, please test There have been no further reports of problems so I am marking this bug resolved. |