Closed Bug 1049738 Opened 6 years ago Closed 6 years ago

Mark toolkit/components/places as FAIL_ON_WARNINGS

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file, 1 obsolete file)

The places code is nearly warning-free. Let's mark it as FAIL_ON_WARNINGS after we've squashed the few remaining warnings.
Depends on: 1049747
Attached patch fix v1 (obsolete) — Splinter Review
This adds a FAIL_ON_WARNINGS annotation towards the bottom of the file, alongside FINAL_LIBRARY, which is approximately where it normally goes.

(It's also indented, because most of this file is indented, due to being inside a "if CONFIG['MOZ_PLACES']" check.)

Here's a preliminary Try run showing that this does indeed make us treat warnings as errors in this directory (as shown by the few red builds): https://tbpl.mozilla.org/?tree=Try&rev=ee64dfe311e2
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8468702 - Flags: review?(mak77)
Attached patch fix v1aSplinter Review
(added a newline before the inserted line, for beautification)
Attachment #8468702 - Attachment is obsolete: true
Attachment #8468702 - Flags: review?(mak77)
Attachment #8468705 - Flags: review?(mak77)
Attachment #8468705 - Flags: review?(mak77) → review+
Try run, with this patch & patches to fix all of the warnings from dependent bugs here:
 https://tbpl.mozilla.org/?tree=Try&rev=4f62d3870219
The Try run from comment 3 looks good, so this is all good to land once bug 1049747 has r+.
[needinfo=me to land this once bug 1049747 can land. I've landed the other blocking bugs.]
Flags: needinfo?(dholbert)
No longer depends on: FAIL_ON_WARNINGS
(Not sure why you bumped 557566 from "depends on" to "blocks" -- we've been marking FAIL_ON_WARNINGS-annotation-additions as "depends on" the FAIL_ON_WARNINGS feature-bug, in the sense that these newer annotations wouldn't be possible without that bug.

I'm adding it back to the "depends on" field. Please don't move it.)
No longer blocks: FAIL_ON_WARNINGS
Depends on: FAIL_ON_WARNINGS
(Looking at the blocking/depends list on bug 557566, it looks like we're a bit inconsistent at the moment, but I think making newer bugs "depend on" that one is more sensible, and that does seem to be the more popular choice.)
Flags: needinfo?(dholbert)
I just landed the fix for the last build warning that I know of in this directory -- bug 1049747.

Here's a sanity-check Try run, with this layered on top of this, to be sure this is good to land:
 https://tbpl.mozilla.org/?tree=Try&rev=4e3bbb0c0bd1
Landed this bug's fix (the FAIL_ON_WARNINGS annotation):
 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8657abef1d7
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/c8657abef1d7
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.