http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8256
M. de Rooy <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #10 from M. de Rooy <[email protected]> --- QA Comment: Works fine. Code looks good to me. Some minor points, just to be strict for this time :-) Routine get_saved_report: Removing the my ($id)=@_ by the more obscure $#_ and $_[0] stuff is no improvement. (Although I understand your motivation..) You are adding several cuddled else's. Like: } else. Not a best practice.. You reduce one line, but also readability. Please use two lines. Just wondering about the use of selector->{id}==0. Could you explain? svc/report line 53-59: First, thanks for commenting! What would be the cost of storing both in the cache BTW? Note that PBP recommends breaking the line before the = operator, and even before reaching the 78 coulmns limit (line 57). About the extended end-of-line comments: For readability, I would prefer here two comment lines starting under the if. (Don't know if that is a PBP or a personal opinion ;) No reasons to block this patch however. Just working on my own PBP knowledge.. Passed QA -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
