http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7417
--- Comment #32 from Jonathan Druart <[email protected]> --- (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 - 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"} - You have to check the returned value of get_from_id in AuthoritiesMarc::GetAuthority: add a 'return unless $authority' seems to be sufficient - 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. -- 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/
