http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5911 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #27 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- (In reply to comment #25)
QA comments: * perlcritic problem: C4/HoldsQueue.pm: Don't modify $_ in list functions at line 571, column 27. See page 114 of PBP. (Severity: 5)
perlcritic can be such a pain...
* why is this code in this patch? +sub _flush_preferences { + %sysprefs = (); +}
It is a poor man's sysprefs flush as per the comment above the code: # FIXME: running this under mod_perl will require a means of # flushing the caching mechanism I did not want to remove the comment because: a) this solution was created in anger, so may not fit all b) I did not address mod_perl/plack/ any other persistant env, just needed a way to change sysprefs for the test
* we're supposed not to add anything to C4::, but use Koha:: instead. You're creating HoldsQueue.pm. As we haven't defined a clear organization for Koha::, I can't say where this code should be, it's probably fair to have it in C4. Plus I don't want Koha:: be filled with C4:: like scripts, so won't reject the patch for this reason
When I started this, I did not even know of Koha namespace. Now I know, but not sure how to put things there :)
The line: # XXX GetHoldsQueueItems() does not support $itemtypeslimit!
should be # FIXME GetHoldsQueueItems() does not support $itemtypeslimit!
I was not sure if that needs addressing, just wanted to add a note. I can change this if required.
Just a question, not related to QA: if the syspref is OFF, how are things sorted ? as before ? (if yes, good point you've set the value to 0 by default, it means there won't be any change for users)
Yes, if off it behaves as before. Default is 0. t/db_dependent/HoldsQueue.t tests both cases. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.