https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35659 --- Comment #41 from Julian Maurice <julian.maurice@biblibre.com> --- Comment on attachment 162693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162693 Bug 35659: OAI Harvester Review of attachment 162693: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=35659&attachment=162693) ----------------------------------------------------------------- Overall this is working really well. OAI repositories mentioned in the test plan can take some time to respond (or fail with a timeout) but this is handled well. However, there are some minor things to fix (see the rest of the review). ::: Koha/OAI/Client/Harvester.pm @@ +79,5 @@
+C<$force> force harvesting (ignore records datestamps) + +=cut + +sub new {
The `new` method doesn't instantiate an object. Instead it sets global variables and returns the class name. I think it's confusing, as all other `new` methods in Koha instantiate an object. @@ +142,5 @@
+ + if ($days) { + + # Change this to yyyy-mm-dd + my $dt_today = dt_from_string();
This variable is not used anywhere. @@ +312,5 @@
+ $imported_record->update( + { + datestamp => $imported_record->datestamp, + } + );
This looks like it does nothing (set "record's datestamp" to "record's datestamp"). Shouldn't it be $oai_record->datestamp (or now if incoming oai record does not have a datestamp) ? -- You are receiving this mail because: You are watching all bug changes.