Description
Jonathan Druart
2011-08-19 09:46:12 UTC
Created attachment 5056 [details] [review] Proposed patch $ git-bz apply 6751 Bug 6751 - Link in readingrec.pl to export a barcode file Proposed patch Apply? [yn] y Applying: Bug 6751: Adds a link "Export checkin barcodes" in readingrec error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt:31 error: koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt: patch does not apply error: patch failed: members/readingrec.pl:29 error: members/readingrec.pl: patch does not apply Patch failed at 0001 Bug 6751: Adds a link "Export checkin barcodes" in readingrec When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Patch left in /tmp/Proposed-patch-dq1fbK.patch Forget it, applies well with a 3-way merge in git bz (In reply to comment #2) Does not apply to current master (using git am -i3u) Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 6751: Adds a link "Export checkin barcodes" in readingrec Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt Auto-merging members/readingrec.pl Failed to merge in the changes. Patch failed at 0001 Bug 6751: Adds a link "Export checkin barcodes" in readingrec When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Created attachment 7012 [details] [review] rebased patch Created attachment 7130 [details] [review] Bug 6751: Adds a link "Export checkin barcodes" in readingrec to export a file containing a list of checkin barcode for the current day. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> QA comment: perlcritic failure. * perlcritic is OK before the patch * after the patch: perlcritic members/readingrec.pl I/O layer ":utf8" used at line 95, column 5. Use ":encoding(UTF-8)" to get strict validation. (Severity: 5) Created attachment 7211 [details] [review] Bug 6751 Remplace encoding mode for perlcritic This patch replaces binmode( STDOUT, ":utf8" ); with binmode( STDOUT, ":encoding(UTF-8)"); The patch does not apply anymore, because of a conflict with Datatables patch that has been pushed (bug 6836) Created attachment 7561 [details] [review] Rebased and Squashed version Bug 6751: Adds a link "Export checkin barcodes" in readingrec to export a file containing a list of checkin barcode for the current day. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> QA comment: * the code itself is OK * percritic still OK after patch applied One thing that make me mark "failed QA" though: 1- the description is "Export checkin barcodes". I can't see anywhere that it's related to "today" checked-in barcode. We will get new bug entries for this "bug". Please rename to "Export today checkin barcodes" & add some information to online help (koha-tmpl/intranet-tmpl/prog/en/help/members/readingrec.pl) 2- Last comment: the link is alone on readingrec.pl. I think it would be better to add it as an option in the "More" toolbar. Marking failed QA for #1, but as you'll submit a new patch, please also fix #2, it will be more graphically consistent Created attachment 7651 [details] [review] Bug 6751 Followup Move export action into the More toolbar This patch move the export into the "more" toolbar. We can export from the following pages: circ/circulation.pl members/notices.pl members/readingrec.pl Created attachment 8001 [details] [review] Bug 6751: Adds a link "Export checkin barcodes" in readingrec to export a file containing a list of checkin barcode for the current day. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Created attachment 8002 [details] [review] Bug 6751: Followup: Move export action into the "More" toolbar Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Also corrected grammar: changed caption from "Export today checkin barcodes" to the more idiomatic "Export today's checked in barcodes." $delimiter is defined as ":", then immediately redefined as "\n". This seems unintentional... Also a small grammatical error in the followup patch: You can export the today checkin barcodes by... Easy enough fixes, but marking as Failed QA until they're resolved. Created attachment 8059 [details] [review] Bug 6751 Followup minors fixes Created attachment 8124 [details] [review] [SIGNED-OFF] Bug 6751: Adds a link "Export checkin barcodes" in readingrec to export a file containing a list of checkin barcode for the current day. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Fixes issues I noticed in QA. Marking as Passed now. |