Closed
Bug 1133846
Opened 9 years ago
Closed 9 years ago
ActivateTimeoutTick logging is missing arguments
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: jdm, Assigned: tvaleev, Mentored)
Details
(Whiteboard: [lang=c++][good first bug])
Attachments
(1 file, 1 obsolete file)
1.07 KB,
patch
|
tvaleev
:
review+
tvaleev
:
checkin+
|
Details | Diff | Splinter Review |
In nsHttpConnetionMgr.cpp, the logging call in ActivateTimeoutTick contains %p references but doesn't have any corresponding arguments. This makes the logs from https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging contain confusing and incorrect data. We should add the missing arguments.
Reporter | ||
Comment 1•9 years ago
|
||
Typo: nsHttpConnectionMgr.cpp
Assignee | ||
Comment 2•9 years ago
|
||
Hi. I created the patch. Is it good?
Reporter | ||
Comment 3•9 years ago
|
||
Looks good to me! If you follow the steps at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_reviewed, you can select a reviewer from the network team at https://wiki.mozilla.org/Modules/Core#Necko or you can use the `suggested reviewers` link that appears when you set the `review` flag to `?`.
Assignee | ||
Comment 4•9 years ago
|
||
Thank you. But could you please advise me... When I change `review` flag to `?` I see 7 suggested people who can review the patch. I selected network module owner Patrick McManus. Am I right?
Flags: needinfo?(josh)
Assignee | ||
Updated•9 years ago
|
Attachment #8565894 -
Flags: review?(mcmanus)
Comment 6•9 years ago
|
||
Comment on attachment 8565894 [details] [diff] [review] 1133846_fix_missing_arguments_v0.1.patch Review of attachment 8565894 [details] [diff] [review]: ----------------------------------------------------------------- Timur - thanks for the patch!
Attachment #8565894 -
Flags: review?(mcmanus) → review+
Comment 7•9 years ago
|
||
protip: you can put the reviewer in your comment line (e.g. r=mcmanus) assuming you get an r+ even when you are just requesting the review .. that way you don't need to change the patch before going to checkin-needed also, to use checkin-needed the sheriffs require try runs. try is great - but it requires L1 hg access. If you don't have that yet I'm happy to vouch for anybody that is posting networking patches. https://www.mozilla.org/en-US/about/governance/policies/commit/
Comment 8•9 years ago
|
||
this is a try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=05a734c31733
Assignee | ||
Comment 9•9 years ago
|
||
Thank you for these tips, Patrick. Yes I don't have L1 hg access (as I understand its Try/User/Incubator Access). But it will be interesting for me to start participate in networking programming. I filed a bug in the "Repository Account Requests" component. So if it possible please vouch for me :)
Assignee | ||
Comment 10•9 years ago
|
||
Oh sorry Patrick. I forget add a link to the bug. Here it is https://bugzilla.mozilla.org/show_bug.cgi?id=1134220.
Assignee | ||
Comment 11•9 years ago
|
||
Hi Patrick. Looks like the pacth is ok. Should I do additional steps to commit it?
Flags: needinfo?(mcmanus)
Comment 12•9 years ago
|
||
you can use the checkin-needed keyword on the bug to have r+'d patches with green try runs committed to the tree on your behalf. to do so, you need to re-upload it with a reviewer name in the description.. in this case it would look like: Bug 1133846 - Add missing arguments to logging call in ActivateTimeoutTick. r=mcmanus when you do that you can set the r+ flag on the "new" patch yourself. that's only controlled by the honor system.
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #8565894 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8568425 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8568425 -
Flags: checkin+
Assignee | ||
Comment 14•9 years ago
|
||
Hi Patrick! I've done additional steps. Am I right?
Flags: needinfo?(mcmanus)
Comment 16•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd1c4a3ba4e3
Assignee: nobody → tvaleev
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fd1c4a3ba4e3
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•