Closed
Bug 841899
Opened 13 years ago
Closed 13 years ago
Remove stream-based logging from Signaling code
Categories
(Core :: WebRTC: Signaling, defect, P3)
Core
WebRTC: Signaling
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: ehugg, Assigned: ehugg)
Details
(Whiteboard: [WebRTC], [blocking-webrtc-], [qa-])
Attachments
(1 file, 2 obsolete files)
|
42.07 KB,
patch
|
ehugg
:
review+
|
Details | Diff | Splinter Review |
The signaling code has some stream-based logging calls such as CSFLogDebugS which are currently only active in Debug builds. This can be confusing when trying to analyze logs from Release builds. We need to replace all of these with their printf-style equivalents, e.g. CSFLogDebug.
| Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ethanhugg
| Assignee | ||
Updated•13 years ago
|
Severity: normal → minor
Priority: -- → P3
Updated•13 years ago
|
Whiteboard: [WebRTC], [blocking-webrtc-]
| Assignee | ||
Comment 1•13 years ago
|
||
| Assignee | ||
Updated•13 years ago
|
Attachment #716231 -
Attachment is obsolete: true
| Assignee | ||
Comment 2•13 years ago
|
||
| Assignee | ||
Updated•13 years ago
|
Attachment #716245 -
Flags: review?(rjesup)
Updated•13 years ago
|
Attachment #716245 -
Flags: review?(rjesup) → review+
| Assignee | ||
Comment 3•13 years ago
|
||
| Assignee | ||
Comment 4•13 years ago
|
||
Comment on attachment 716606 [details] [diff] [review]
Remove stream-based logging from Signaling code
Re-based patch on latest M-I because of merge conflicts with Bug 841641
Bringing forward r+ from Jesup
Try is here:
https://tbpl.mozilla.org/?tree=Try&rev=cdc2c22a6901
Attachment #716606 -
Flags: review+
| Assignee | ||
Updated•13 years ago
|
Attachment #716245 -
Attachment is obsolete: true
| Assignee | ||
Comment 5•13 years ago
|
||
Try run with -p all instead of -p none...
https://tbpl.mozilla.org/?tree=Try&rev=b8510aa237e7
| Assignee | ||
Comment 6•13 years ago
|
||
Comment 7•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•13 years ago
|
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-], [qa-]
Comment 8•13 years ago
|
||
What was gained here? Varargs are prone to error, so this seems like a step backwards.
| Assignee | ||
Comment 9•13 years ago
|
||
>What was gained here? Varargs are prone to error, so this seems like a step >backwards.
Streams are not allowed in Mozilla production code. So all of these instances that were just changed were #ifdef DEBUG which meant that some logging was on opt builds and some not. Now it should be less confusing when trying to debug an opt build.
I agree with you of course that the streams versions would've been better to use and less prone to error in all of this CPP code and that's why they existed, but streams bad because Mozilla.
You need to log in
before you can comment on or make changes to this bug.
Description
•