https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15494 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + unless ($item) { carp "No item passed in!" && return } Not our preferred coding style + return () if (! $yaml =~ /\S/ ); Just return here. The only call is in scalar context. We do not need a list. Shorter: return if $yaml !~ /\S/; + eval { + $denyingrules = YAML::Load($yaml); + }; How much time are we spending on doing this? What is the performance effect? If the pref always looks like field1: [val]\nfield2: [val2] we can just split on newline and split on ':', etc. Give me some convincing numbers please. + return (); Same as above + my $val; + if (exists $item->{$field}) { + $val = $item->{$field}; + } + $val = '' unless defined $val; Why not in just one line: my $val = $item->{$field} // ''; + - How to write these rules is documented on the <a href="http://wiki.koha-community.org/wiki/OpacHiddenItems" target="_blank">Koha wiki</a>. This is from the preference file. Not sure if this is the preferred way. A wiki page could be gone just as easily. Why not just provide an example as close as possible to the pref itself. Just another 40 chars ? Just curious too if this supports denying renewal on an empty string? If so, the conversion from undef to empty string might not be what we want. Can we signal empty string or undef to be a blocking value? [No blocker for me btw :)] Changing status. Please make some adjustments. -- You are receiving this mail because: You are watching all bug changes.