[Koha-bugs] [Bug 9032] Share a list

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #71 from M. de Rooy <m.de.rooy at rijksmuseum.nl> ---
(In reply to Jonathan Druart from comment #70)
> QA comment:
Thanks for taking a look.

> 1/ In AddShare: you remove the 10 oldest entries and add 1? What I missed?
Just housekeeping. 10 is just an arbitrary number. (Could add this at some
point in cleanup_database.)

> 2/ AddShare:
>     return !$dbh->err;
>     => useless, $dbh->do insert returns the number of inserted rows.
Not sure. Could that be 0E0 ? Think that this is safer.

> 3/ C4::VirtualShelves::ShelfPossibleAction
> Why do you use numeric code? It should be better to use an readable code.
> Moreover, these codes are used in the template.
Have been using error numbers in shareshelf everywhere. Would recommend to
leave it as-is for now.

> 4/  return if !defined($i) || !$i || $i eq '0E0'; #not found
>     return 1;
> Could be replaced with
>  return 1 if $i > 0
> I think.
Not sure about that too.

> 6/ (In reply to M. de Rooy from comment #31)
> > (In reply to Jonathan Druart from comment #26)
> > > - The new letter should be added to others languages.
> > I would really get rid of that restriction. I started some development on
> > that (look for i18n). But unfortunately there is not that much support for
> > it.
> Yes, but this one cannot be pushed as it and this one is not marked as
> dependent on the other bug report.
I abandoned the i18n reports. Getting that into Koha is too much for me on my
own. Even reports of this size are hard to get in.
In the commit message I wrote that I will add the other 10? languages as soon
as this is ready for a push. (It will only trigger more merge conflicts.)

> 7/
> opac/opac-shareshelf.pl
> + for( 0..$#$alphabet )
> 
> It could be good to name the variable loop.
No big deal. Will do that.

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