Closed
Bug 891993
Opened 11 years ago
Closed 11 years ago
Mark toolkit/components/downloads/ as FAIL_ON_WARNINGS
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
762 bytes,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
The directory toolkit/components/downloads/ is build-warning-free on all supported platforms. Filing this bug to mark it as FAIL_ON_WARNINGS, to keep it that way. Here's a fully-green Try run, with the annotation added, to prove that this is safe: https://tbpl.mozilla.org/?tree=Try&rev=53b34881aba0
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
(Aside: on my local machine, this directory does hit a few instances of "-Wdeprecated-declarations" warnings (for using deprecated GTK APIs), but those won't cause problems here because we've already excepted -Wdeprecated-declarations from FAIL_ON_WARNINGS, over in bug 833405.)
Comment 3•11 years ago
|
||
Comment on attachment 773420 [details] [diff] [review] patch v1 Review of attachment 773420 [details] [diff] [review]: ----------------------------------------------------------------- I honestly trust your checks and the tryserver here. Since I don't have anything against fail on warning, it's fine.
Attachment #773420 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Thanks! Side note: I verified locally that this was being enforced -- I added an unused variable to one of the .cpp files in this directory, and it caused a build error (as you'd expect).
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4df2e13d4157
Flags: in-testsuite-
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4df2e13d4157
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 7•11 years ago
|
||
Hey Daniel, I introduced a protocol buffer library as a dependency in https://bugzilla.mozilla.org/show_bug.cgi?id=837199 that is not compatible with this change. A lot of this code is automatically generated by the protocol buffer compiler and the bug to fix this has been open for more than a year: http://code.google.com/p/protobuf/issues/detail?id=361&colspec=ID%20Type%20Status%20Priority%20FixedIn%20Owner%20Summary Is it possible to revert this? Monica
Updated•11 years ago
|
Flags: needinfo?(dholbert)
Assignee | ||
Comment 8•11 years ago
|
||
Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/7645c1ac2ec3 so as not to block Monica from landing the 3rd-party header mentioned in comment 7 ( csd.pb.h ), which apparently has a few build warnings. Per irc discussion, Monica's going to file a bug on those warnings, blocking this one, so that once we (hopefully) address them (or once upstream addresses them), we can re-land this annotation.
Status: RESOLVED → REOPENED
Flags: needinfo?(dholbert) → needinfo?(mmc)
Resolution: FIXED → ---
Assignee | ||
Comment 9•11 years ago
|
||
Actually, from skimming the build logs from the Try run for bug 837199, I'm not convinced that protobuf is actually triggering any build warnings in toolkit/components/downloads/ ... The code.google.com issue flagged in comment 7 seems to be for warnings in protobuf .cc files, and some of those are shown in the Try run's build logs, but those files don't live in toolkit/components/downloads/, so they shouldn't cause any trouble here. Maybe I'm missing something? If not, then we don't need to file a new bug (blocking this one) after all, and this never actually needed to be backed out & can be re-landed.
Comment 10•11 years ago
|
||
Hi Daniel, Thanks for helping me :) I am recompiling with my local fixes and your original patch: https://tbpl.mozilla.org/?tree=Try&rev=356a26b7fa58 Monica
Flags: needinfo?(mmc)
Comment 11•11 years ago
|
||
Oops, leftover mtype: https://tbpl.mozilla.org/?tree=Try&rev=5f8239555046
Assignee | ||
Comment 12•11 years ago
|
||
Per IRC discussion with Monica, she agrees that Comment 9 is probably right -- the only build warnings introduced in this dir in Bug 837199 were in Mozilla code, and Monica's fixed those locally. So, I re-landed this: https://hg.mozilla.org/integration/mozilla-inbound/rev/c434fcf6825b
Comment 13•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c434fcf6825b
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•