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

--- Comment #5 from M. Tompsett <[email protected]> ---
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.
_______________________________________________
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/

Reply via email to