http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7417
--- Comment #33 from Jared Camins-Esakov <[email protected]> --- (In reply to comment #32) > (In reply to comment #31) > > Jonathan, > > > > I think the thing to do here is move forward with the Class::Accessor-based > > code. While I am willing to rewrite this to use Moo, I think that it might > > be a bit premature to do that at this point. Better, I think, to make sure > > that there's consensus on using Moo as a compromise framework first, per > > comment 19. In the meantime, if you don't mind I would like to incorporate > > any further QA comments you might have so that this patch can move forward. > > Jared, > > All seems good. > To be perfect, the following should be made: > > - pass perltidy on new files Will do. > - for discussion: Isn't it possible to have an "intelligent" constructor for > Authorities ? I think we don't want a lot of get_from_*. Perhaps a "new" > routine with a test on param: > either: new( { record => $record} ), new ({authid => $authid}) > or: new ($var). sub new {my $self = shift; if (ref $var eq > 'MARC::Record') { print "it's a record" } else { print "it's an authid"} We could certainly do this. My original implementation used the former option, but for some reason I changed it. I don't remember quite why. > - You have to check the returned value of get_from_id in > AuthoritiesMarc::GetAuthority: > add a 'return unless $authority' seems to be sufficient Will do. > - I don't understand what you do exactly with the indicator: > Your comment: > +In order to differentiate added headings from actual headings, a 'z' is > +put in the first indicator. > I am not an UNIMARC or *MARC* expert but isn't the content of this > indicator normalized ? > I don't manage to find if it is a work around or the best way to do that. Yes. 'z' is always an invalid indicator value, because the standard calls for a numeric value. So any time we find a letter in an indicator, we can safely assume that the field is one that should not be displayed or used other than for searching. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
