Bug 9144

Summary: bulkmarcimport.pl - Problem identifying errors
Product: Koha Reporter: Vitor Fernandes <vfernandes>
Component: MARC Bibliographic record staging/importAssignee: Vitor Fernandes <vfernandes>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: trivial    
Priority: P1 - high CC: chris, kyle, paul.poulain
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: BUG 9144
Bug 9144 - bulkmarcimport.pl - Problem identifying errors
Bug 9144 - bulkmarcimport.pl - Problem identifying errors

Description Vitor Fernandes 2012-11-26 10:47:21 UTC
The bulkmarcimport.pl uses replace instead of changing line, which difficults the track of the errors found in the migration.

Solution: replace \r to \n
Comment 1 Vitor Fernandes 2012-11-26 11:24:33 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-12-07 12:40:01 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-12-07 12:41:37 UTC
We use \n for newlines everywhere else in Koha, so this is a sensible change.
Comment 4 Paul Poulain 2012-12-12 08:04:36 UTC
QA comment: one letter patch, passes QA !
Comment 5 Paul Poulain 2012-12-12 08:05:03 UTC
Created attachment 14027 [details] [review]
Bug 9144 - bulkmarcimport.pl - Problem identifying errors

Replace \r with \n for newline in output for bulkmarcimport.pl

\r is the newline character for OS X, \n is the newline character
for Linux.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 6 Jared Camins-Esakov 2012-12-12 13:27:00 UTC
I am inclined to push this patch, since I agree with Vitor's assessment that it would probably be easier to spot errors when using \n than when using \r. However, the sign off and QA appear to be predicated on the idea that this patch increases consistency, which it does not do. In light of this, I would request that Kyle and Paul confirm whether they would like their sign off and QA (respectively) to stand.
Comment 7 Kyle M Hall 2012-12-12 14:44:15 UTC
(In reply to comment #6)
> I am inclined to push this patch, since I agree with Vitor's assessment that
> it would probably be easier to spot errors when using \n than when using \r.
> However, the sign off and QA appear to be predicated on the idea that this
> patch increases consistency, which it does not do. In light of this, I would
> request that Kyle and Paul confirm whether they would like their sign off
> and QA (respectively) to stand.

I stand by my SO, for ease of readability. I suppose if we want full consistency, we would need to find every \r and \n in Koha and change them to \r\n.
Comment 8 Paul Poulain 2012-12-17 16:50:18 UTC
(In reply to comment #6)
> I am inclined to push this patch, since I agree with Vitor's assessment that
> it would probably be easier to spot errors when using \n than when using \r.
> However, the sign off and QA appear to be predicated on the idea that this
> patch increases consistency, which it does not do. In light of this, I would
> request that Kyle and Paul confirm whether they would like their sign off
> and QA (respectively) to stand.

Jared, I don't understand what you're saying. (ie= I don't see where I said this patch improves consistency)

back to passed QA, this patch is OK for me
Comment 9 Jared Camins-Esakov 2012-12-17 16:54:06 UTC
This patch has been pushed to master.

(Paul, you didn't say explicitly that you were QAing the patch with the idea that it improved consistency, but that was implied by the commit message and I wanted to make sure you wholly approved)
Comment 10 Chris Cormack 2012-12-17 21:43:58 UTC
Pushed to 3.10.x will be in 3.10.1