http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7417 --- Comment #33 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- (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.