http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11368 --- Comment #13 from Kyle M Hall <kyle.m.hall@gmail.com> --- (In reply to Jonathan Druart from comment #11)
Comment on attachment 39182 [details] [review] Bug 11368 - Add script to import Lexile scores
Review of attachment 39182 [details] [review]: -----------------------------------------------------------------
I get this warning: DBIx::Class::ResultSet::search_rs(): search( %condition ) is deprecated, use search( \%condition ) instead at misc/migration_tools/import_lexile.pl line 147
Fixed!
::: misc/migration_tools/import_lexile.pl @@ +65,5 @@
+ 'source=s' => $subfield_source, + 'source-value=s' => $subfield_source_value, +); + +my $usage = << 'ENDUSAGE';
Should not we use Pod::Usage?
@@ +82,5 @@
+ +ENDUSAGE + +unless ($file) { + say $usage;
Prefer to call pod2usage.
I don't think the developer guidelines require pod2usage. That being said, I'd welcome a followup!
@@ +88,5 @@
+} + +my $schema = Koha::Database->new()->schema(); + +my $csv = Text::CSV->new( { binary => 1, sep_char => "\t" } )
Shouldn't we use the pref "separator" and add an option to specify another one?
The file format is fixed with tabs as the separator, so we should definitely not use the syspref here.
@@ +121,5 @@
+ push( @isbns, $row->{$_} ); + eval { push( @isbns, GetVariationsOfISBN( $row->{$_} ) ) }; + } + } + @isbns = grep( $_, @isbns );
Not sure to undestand what this does :)
This line filters out any 'empty' isbns.
@@ +142,5 @@
+ + if ($verbose) { + say "Found matching record! Biblionumber: $biblionumber"; + + if ( $verbose > 2 ) {
You should mention this in the POD.
Done! -- You are receiving this mail because: You are watching all bug changes.