[Koha-bugs] [Bug 33608] Allow to get statistics about found/recovered books

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33608

Tomás Cohen Arazi <tomascohen at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #79 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
I generally like the cleanup being made in the area. Great job!

I have some QA remarks that I consider blockers. Sorted randomly.

1) Changes to POD in Koha::Statistic are wrong in terms of consistency:

-=head2 Class methods
+=head2 METHODS
...
-=head2 Internal methods
+=head2 INTERNAL METHODS

Please keep the 'style' we picked some years ago.

2) Koha::Statistic->_key_or_default feels bad. I'd prefer a ternary operator.

3) Koha::Statistic->insert means introducing a new pattern, and I don't think
we need it. I'd ask you to file a new bug for proposing that pattern and have
devs discuss it there, so this enhancement is not blocked.

4) C4/Stats.pm =>

+use Koha::Statistic;

^^ it should use the plural class.

5) Probably for Martin's follow-up: if we are tidying an entire hash structure,
I usually suggest we sort keys on the same run. I can do it inline, but thought
it was worth mentioning explicitly.

6) This would benefit from some patch squashing.

I'm happy to push this ASAP once the issues are solved.

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