[Koha-bugs] [Bug 13967] System preferences need a package

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Wed Apr 8 15:35:32 CEST 2015


http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13967

--- Comment #5 from M. Tompsett <mtompset at hotmail.com> ---
Comment on attachment 37576
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37576
Bug 13967 - System preferences need a package

Review of attachment 37576:
 --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13967&attachment=37576)
-----------------------------------------------------------------

::: C4/Context.pm
@@ +106,5 @@
>  use Module::Load::Conditional qw(can_load);
>  use Carp;
>  
> +use C4::Boolean;
> +use C4::Debug;

Why move these two lines?

::: Koha/Objects.pm
@@ +84,4 @@
>  
>      my $result = $self->_resultset()->find($id);
>  
> +    return unless $result;

This is a behaviour we want for backward compatibility with preference(), but
is this a correction? If so, perhaps this is a separate bug fix, technically?
More scope creep?

::: Koha/SysPrefs.pm
@@ +28,5 @@
> +use base qw(Koha::Objects);
> +
> +=head1 NAME
> +
> +Koha::SysPrefs - Koha Borrower Object set class

Cut and paste issues? What does Borrower got to do with anything?

::: t/db_dependent/sysprefs.t
@@ +27,4 @@
>  $dbh->{RaiseError} = 1;
>  $dbh->{AutoCommit} = 0;
>  
> +my $opacheader    = C4::Context->preference('opacheader');

Good thing this is small. Perl tidying should be a separate patch.

@@ +31,4 @@
>  my $newopacheader = "newopacheader";
>  
> +C4::Context->set_preference( 'OPACHEADER', $newopacheader );
> +is( C4::Context->preference('opacheader'), $newopacheader );

This is the better way, in my opinion, to test. ok() with 'eq' type compatisons
is asking for trouble. However, if the scope of this bug is to add
Koha::SysPref(s), then this is scope creep.

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list