http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14504
--- Comment #5 from Jonathan Druart <[email protected]> --- Comment on attachment 42978 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42978 [SIGNED-OFF] Bug 14504: Add delete_items.pl: a command line batch deletion tool Review of attachment 42978: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14504&attachment=42978) ----------------------------------------------------------------- FAIL misc/cronjobs/delete_items.pl FAIL pod *** WARNING: Verbatim paragraph in NAME section in file misc/cronjobs/delete_items.pl ::: misc/cronjobs/delete_items.pl @@ +1,4 @@ > +#! /usr/bin/perl > + > +use warnings; > +use strict; warnings and strict are useless when Modern::Perl is already used. @@ +8,5 @@ > +use C4::Circulation; > +use Modern::Perl; > +use Pod::Usage; > + > +my $VERSION='1.0'; How this is useful? We usually don't use it in scripts. @@ +30,5 @@ > + , help => '' > + , manual => '' > + , version => '' > + } > +}; I am not really in favor of these 2 variables. IMO it is preferable to stick to the structure of the already existing scripts in misc/* @@ +39,5 @@ > + , 'V|version' => sub { $OPTIONS->{flags}->{version} = 1 } > + , 'h|help' => sub { $OPTIONS->{flags}->{help} = 1 } > + , 'm|manual' => sub { $OPTIONS->{flags}->{manual} = 1 } > + , 'c|commit' => sub { $OPTIONS->{flags}->{commit} = 1 } # aka > DO-EET! > + , 'dry-run' => sub { $OPTIONS->{flags}->{commit} = 0; I don't think dry-run is useful, it's in dry-run mode if commit is not given. @@ +51,5 @@ > + exit; > +} > + > +pod2usage( -verbose => 2 ) if $OPTIONS->{flags}->{manual}; > +pod2usage(1) if ( $OPTIONS->{flags}->{help} || scalar @criteria == 0 ); The script should not return 1 if help is specified. It would be good to display on error for the @criteria==0 case (see msg option of pod2usage). @@ +65,5 @@ > +my $where_clause = ' where ' . join ( " and ", @criteria ); > + > +verbose "Where statement: $where_clause"; > + > +$GLOBAL->{sth}->{target_tiems} = $dbh->prepare( $query->{target_items} . > $where_clause ); typo tiems vs items, I suppose. @@ +71,5 @@ > + > +DELITEM: while ( my $item = > $GLOBAL->{sth}->{target_tiems}->fetchrow_hashref() ) { > + my $issue = GetOpenIssue( $item->{itemnumber} ); > + if( defined $issue ) { > + verbose "Cannot delete '$item->{itemnumber}' -- item is checked out." Shouldn't we also search for holds? -- 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/
