Last Comment Bug 756448 - [compare-locales] merge content wrong with more than one error per file
: [compare-locales] merge content wrong with more than one error per file
Status: RESOLVED FIXED
:
Product: Mozilla Localizations
Classification: Client Software
Component: Infrastructure (show other bugs)
: unspecified
: All All
: -- major (vote)
: ---
Assigned To: Axel Hecht [:Pike]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-18 06:37 PDT by Axel Hecht [:Pike]
Modified: 2012-06-07 04:27 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
sort skips by start offset (578 bytes, patch)
2012-05-18 06:37 PDT, Axel Hecht [:Pike]
gandalf: review+
Details | Diff | Splinter Review

Description Axel Hecht [:Pike] 2012-05-18 06:37:50 PDT
Created attachment 625083 [details] [diff] [review]
sort skips by start offset

The entries for skips in ContentComparer.merge were ordered per key, and not by file location. That means, the chunks to be removed for errornous values weren't sequenced, and thus the resulting file would be, say, non-optimal.

The fix is easy, just sort the thing by start offset.

I verified this by running over a few merge outputs which have errors on fx_central.
Comment 1 Axel Hecht [:Pike] 2012-05-24 05:29:51 PDT
Comment on attachment 625083 [details] [diff] [review]
sort skips by start offset

Gandalf said he'd take a look at this, requesting review.
Comment 2 Zibi Braniecki [:gandalf][:zibi] 2012-05-25 03:43:28 PDT
Comment on attachment 625083 [details] [diff] [review]
sort skips by start offset

Review of attachment 625083 [details] [diff] [review]:
-----------------------------------------------------------------

looks and works good :)
Comment 3 Axel Hecht [:Pike] 2012-06-07 04:27:49 PDT
http://hg.mozilla.org/l10n/compare-locales/rev/a36840ad87a1, FIXED.

Note You need to log in before you can comment on or make changes to this bug.