http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11175 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi, starting with a code review: - Small thing: Please break your commit messages into shorter lines so they are easier to read in a terminal window. - Please provide a follow-up for the Bootstrap theme. As prog and CCSR are going to be deprecated this is mandatory now. - Database update adding the new system preference is missing. - The way this is coded it's a MARC21 specific feature. The rcn index and 001 > $w linking are not MARC agnostic. Please check for the marcflavor in your code and include a note about the limitation to MARC21 in the syspref description. - Some of the comments look like TODOs - maybe something you want to take another look at? - Creating our 'own' XML in a MARCXML record doesn't look right to me. I would much prefer if that could be expressed in normal MARC21 fields and subfields. - With the heading "Child records" you assume that it will be always a component part, but that's not true. 001 > $w linkings are used to express a lot of different relationships between records. I think putting a bit more thought into the various relationships could be good here. Also the QA script points out some issues: FAIL C4/XSLT.pm FAIL pod *** ERROR: Unknown command 'head' in file C4/XSLT.pm *** ERROR: Spurious text after =cut in file C4/XSLT.pm OK forbidden patterns OK valid FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). -- You are receiving this mail because: You are watching all bug changes.