Closed
Bug 1147217
Opened 9 years ago
Closed 9 years ago
Improve l10n repack error message when locale doesn't contain necessary files
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox38 fixed, firefox39 fixed)
RESOLVED
FIXED
mozilla39
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file, 2 obsolete files)
2.05 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8582765 -
Flags: review?(mshal)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8582786 -
Flags: review?(mshal)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8582765 -
Attachment is obsolete: true
Attachment #8582786 -
Attachment is obsolete: true
Attachment #8582765 -
Flags: review?(mshal)
Attachment #8582786 -
Flags: review?(mshal)
Attachment #8582795 -
Flags: review?(mshal)
Comment 4•9 years ago
|
||
Comment on attachment 8582795 [details] [diff] [review] Improve l10n repack error message when locale doesn't contain necessary files >+ if base not in l10n_paths: >+ errors.fatal("Locale doesn't contain %s/" % base) >+ continue >+ if e.name not in l10n_paths[base]: >+ errors.fatal("Locale doesn't have a manifest entry for '%s'" % >+ e.name) >+ continue Why did you need to add the continues here? It looks like errors.fatal() should raise an exception.
Attachment #8582795 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #4) > Comment on attachment 8582795 [details] [diff] [review] > Improve l10n repack error message when locale doesn't contain necessary files > > >+ if base not in l10n_paths: > >+ errors.fatal("Locale doesn't contain %s/" % base) > >+ continue > >+ if e.name not in l10n_paths[base]: > >+ errors.fatal("Locale doesn't have a manifest entry for '%s'" % > >+ e.name) > >+ continue > > Why did you need to add the continues here? It looks like errors.fatal() > should raise an exception. errors.accumulate(), which is used in the caller, inhibits those exceptions, accumulates the errors, and then throws an aggregated exception with all the errors.
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b23c836ff69a
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b23c836ff69a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 8•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/5c4cacd09c9c
status-firefox38:
--- → fixed
Comment 9•9 years ago
|
||
Comment on attachment 8582795 [details] [diff] [review] Improve l10n repack error message when locale doesn't contain necessary files I thought we could get away without it, but it seems we need to uplift this one for bug 1147207 to cleanly apply. Please see that bug for approval comment.
Attachment #8582795 -
Flags: approval-mozilla-beta?
Comment 10•9 years ago
|
||
Comment on attachment 8582795 [details] [diff] [review] Improve l10n repack error message when locale doesn't contain necessary files Scratch that, you've already uplifted it. Sorry I missed it!
Attachment #8582795 -
Flags: approval-mozilla-beta?
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•