[Koha-bugs] [Bug 10486] Allow external Z39.50 targets to be searched from the OPAC
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Fri May 2 16:11:23 CEST 2014
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10486
--- Comment #83 from Jonathan Druart <jonathan.druart at biblibre.com> ---
So, QA comment just looking at the code:
1/ use Modern::Perl in the opac/svc file
2/ warning
"my" variable $response masks earlier declaration in same scope at
/home/koha/src/opac/svc/pazpar2_init line 80
3/ Why do you create a new table and don't use the entries in z3950servers
(with a new column)?
4/ Some error raised by qa tools:
FAIL koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
FAIL forbidden patterns
forbidden pattern: opac-tmpl should certainly replaced with [% interface %]
(line 276)
And some otheres. You should move new js files in koha-tmpl/intranet-tmpl/lib
(see bug 12119, bug 12116, etc.)
5/ For your next patch, please split it in several small ones. It is very hard
to read a 249.78 KB patch :)
6/ Unit tests are missing for GetExternalSearchTargets
7/ param debug => 1 is useless in get_template_and_user calls
8/ SQL queries in .pl files should be avoided
9/ I didn't understand the new css rules for .highlight
10/ + <input type="checkbox" id="branch-[%
branch.branchcode %]" name="branch" value="[% branch.branchcode %]" checked />
should be checked="checked"
11/ Please squash changes in opac.css (you completely rewrite the file in the
first patch). And now I understand the size of the patch :)
12/ New js lib should be added to the about page, licenses tab.
I don't change the status because I didn't test, but it seems to fail QA.
--
You are receiving this mail because:
You are watching all bug changes.
More information about the Koha-bugs
mailing list