Open
Bug 1302245
Opened 8 years ago
Updated 2 years ago
Fix -Winconsistent-missing-override warnings in media/webrtc/
Categories
(Core :: WebRTC, defect, P5)
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox51 | --- | affected |
People
(Reporter: MatsPalmgren_bugz, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Building media/webrtc/ generates hundreds of warnings of this type:
Warning: -Winconsistent-missing-override in media/webrtc/trunk/webrtc/video_engine/vie_rtp_rtcp_impl.h: 'SetFECStatus' overrides a member function but is not marked 'override'
This is annoying and thus allows other warnings to go unnoticed since
the signal-to-noise ratio is so low.
Reporter | ||
Comment 1•8 years ago
|
||
The patch adds the keyword 'override' where needed.
There are no other changes.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a60594a64ea1
Attachment #8790518 -
Flags: review?(rjesup)
Comment 2•8 years ago
|
||
Mats - this is all imported code -- and we're about to land a major update. Can you wait until update_49 lands, then either propose a patch against that, or file a bug upstream at webrtc.org? Note that the version your patch is against is 43; we're importing 49 right now, and current stable rev is something like 52 or 53. Some of the things in your patch I know are fixed in 49; others I'm sure are not.
Landing fixes for warnings/etc against this code makes imports more painful (conflicts, and we have to uplift the changes). We do it from time to time if it blocks something like static analysis or shows a real possible bug, but we try to keep it to minimum and instead fix upstream, or ignore non-bug warnings in the imported code.
Comment 3•8 years ago
|
||
Comment on attachment 8790518 [details] [diff] [review]
fix
Review of attachment 8790518 [details] [diff] [review]:
-----------------------------------------------------------------
Cancelling review until webrtc/49 lands
Attachment #8790518 -
Flags: review?(rjesup)
Updated•8 years ago
|
Rank: 45
Priority: -- → P4
Reporter | ||
Comment 4•8 years ago
|
||
Sorry, I didn't know this was all imported code. Fixing it upstream instead makes sense.
Assignee: mats → nobody
Comment 5•7 years ago
|
||
Mass change P4->P5 to align with new Mozilla triage process.
Priority: P4 → P5
Updated•2 years ago
|
Severity: minor → S4
You need to log in
before you can comment on or make changes to this bug.
Description
•