Summary: | Be stricter with utf-8 output | ||
---|---|---|---|
Product: | Koha | Reporter: | Colin Campbell <colin.campbell> |
Component: | Architecture, internals, and plumbing | Assignee: | Colin Campbell <colin.campbell> |
Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
Severity: | enhancement | ||
Priority: | P3 | CC: | chris, dubyk, f.demians, jonathan.druart, koha.sekjal, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
See Also: | http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6554 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | |||
Bug Blocks: | 6895 | ||
Attachments: |
Proposed Patch
Signed-off patch Patch rebased against current HEAD Updated patch signed-off patch |
Description
Colin Campbell
2011-08-19 10:42:43 UTC
Created attachment 5058 [details] [review] Proposed Patch Patch attached: tests now pass Created attachment 5252 [details] [review] Signed-off patch I understand Colin concern and approve his solution which is obviously valid from a Perl strictness point of view. I haven't tested all modified scripts in order to detect any side effect, but don't think it's necessary. QA comment : * if it's to fix a perlcritic I won't object * I would have preffered to have included this patch earlier, with opportunity to have more time to test more cases * OTH, Frédéric has also diacritics concern Marking passed QA, but would not object if chris requires more testing ! From what I can tell, this safely implements the stricter UTF-8 checks across numerous affected files; several files (C4/tests/Record_test.pl and C4/Installer.pm, most notably) do not have this change made; they are slurping in UTF-8 *input* however, which is outside the scope of this report. Any replacement of file handles with variables is done universally, so no mis-matches there. Marking as Passed QA I don't feel I have enough time to test this thoroughly, holding for 3.6.1 sorry, but it does not apply anymore against master : 11:08 ~/koha.dev/koha-community (new/bug_6752 $%)$ git am -3 /tmp/Signed-off-patch-OH345M.patch Applying: Bug 6752: Be stricter with utf-8 encoding of output Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging admin/aqplan.pl Auto-merging circ/overdue.pl CONFLICT (content): Merge conflict in circ/overdue.pl Auto-merging circ/ypattrodue-attr-search-authvalue.pl CONFLICT (content): Merge conflict in circ/ypattrodue-attr-search-authvalue.pl Auto-merging circ/ysearch.pl CONFLICT (content): Merge conflict in circ/ysearch.pl Auto-merging misc/migration_tools/rebuild_zebra.pl Auto-merging misc/translator/xgettext.pl CONFLICT (content): Merge conflict in misc/translator/xgettext.pl Auto-merging tools/export.pl Failed to merge in the changes. Patch failed at 0001 Bug 6752: Be stricter with utf-8 encoding of output I planned to test at least some of them before pushing, and will do that once you submit a rebases patch. Created attachment 6248 [details] [review] Patch rebased against current HEAD Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 6752: Be stricter with utf-8 encoding of output Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging cataloguing/ysearch.pl CONFLICT (content): Merge conflict in cataloguing/ysearch.pl Auto-merging misc/cronjobs/overdue_notices.pl Auto-merging misc/migration_tools/rebuild_zebra.pl Failed to merge in the changes. Patch failed at 0001 Bug 6752: Be stricter with utf-8 encoding of output 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 6928 [details] [review] Updated patch Updated patch to remove one case that has already been corrected by a subsequent patch Created attachment 7246 [details] [review] signed-off patch This patch has been tested by many people (the initial one, or the rebased one) Marking as "passed QA", i'll push it in a few seconds. However, I keep it for 3.8. If there is an undetected side-effect, we will have time to see it. chris_n, if you object you can push for 3.6, but I think you should not. Patch pushed. Note that Jonathan forgot to add his signoff to the "signed-off patch" he uploaded, I've added it myself (with my signoff) before pushing. There have been no further reports of problems so I am marking this bug resolved. |