1049738 - Mark toolkit/components/places as FAIL_ON_WARNINGS

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Daniel Holbert [:dholbert]]

The places code is nearly warning-free. Let's mark it as FAIL_ON_WARNINGS after we've squashed the few remaining warnings.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

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

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1a

(added a newline before the inserted line, for beautification)

Comment 3

[Daniel Holbert [:dholbert]]

Try run, with this patch & patches to fix all of the warnings from dependent bugs here:
 https://tbpl.mozilla.org/?tree=Try&rev=4f62d3870219

Comment 4

[Daniel Holbert [:dholbert]]

The Try run from comment 3 looks good, so this is all good to land once bug 1049747 has r+.

Comment 5

[Daniel Holbert [:dholbert]]

[needinfo=me to land this once bug 1049747 can land. I've landed the other blocking bugs.]

Comment 6

[Daniel Holbert [:dholbert]]

(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.)

Comment 7

[Daniel Holbert [:dholbert]]

(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.)

Comment 8

[Daniel Holbert [: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

Comment 9

[Daniel Holbert [:dholbert]]

Landed this bug's fix (the FAIL_ON_WARNINGS annotation):
 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8657abef1d7

Comment 10

[Nigel Babu [:nigelb]]

https://hg.mozilla.org/mozilla-central/rev/c8657abef1d7