[Koha-bugs] [Bug 8133] hourly loans doesn't know when library closed

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Fri Nov 22 23:09:18 CET 2013


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

--- Comment #9 from Jesse Weaver <jweaver at bywatersolutions.com> ---
(In reply to Jonathan Druart from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > Wow - this is huge!
> 
> Yes, it is.
> 
> Jesse, do you think it is possible to divide this patch into smaller ones?
> Creating many iterations will help moving ahead this patch.

Yes, going to separate it into 2 bugs and 2 or 3 patches.

> This patch does not apply anymore, so I just look at the code.
> Here are some notes:
> 1/ Use qa-tools in order to flush out some wrong lines (e.g. Koha is under
> the GPLv3 license, not v2).

Will do.

> 2/ my $hours = { %{ $self->get_hours( $dt ) } };
> is not it the same as $self->get_hours( $dt ) ?

This is used to clone the hash in question.

> 3/ IN DUPLICATE is a Mysqlism.
> 4/ in updatedb: use a transaction to avoid a lost of data.

Good idea, changing this.

> 5/ Please use "git mv" for renaming files (e.g. tools/holidays.tt and
> tools/calendar.tt). The changes in the git history will be easier to read.

I did so, the changes afterwards were too much for git to keep track of. The
separated patches should show the rename if the changes are less drastic.

> 6/ try to provide several small patches (if possible).
> 7/ provide a more detailed test plan

Will do so, but within reason. None of the circulation or overdues UI has been
changed at all (except for a couple minor cronjobs), and testing them
separately from the unit tests for Koha::Calendar accomplishes nothing. The
only changes to C4::Circulation and C4::Overdues were to remove unused subs
that depended on C4::Calendar (this can be verified with a quick grep).

> 
> Marked as Failed QA.

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


More information about the Koha-bugs mailing list