Closed Bug 891993 Opened 7 years ago Closed 7 years ago
Mark toolkit/components/downloads/ as FAIL
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: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #773420 - Flags: review?(mak77)
(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 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+
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).
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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
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 → ---
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.
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
Oops, leftover mtype: https://tbpl.mozilla.org/?tree=Try&rev=5f8239555046
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
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.