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.
Created attachment 8565894 [details] [diff] [review] 1133846_fix_missing_arguments_v0.1.patch Hi. I created the patch. Is it good?
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 `?`.
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?
Yes, that's fine :)
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!
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/
this is a try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=05a734c31733
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 :)
Oh sorry Patrick. I forget add a link to the bug. Here it is https://bugzilla.mozilla.org/show_bug.cgi?id=1134220.
Hi Patrick. Looks like the pacth is ok. Should I do additional steps to commit it?
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.
Created attachment 8568425 [details] [diff] [review] 1133846_fix_missing_arguments_v0.3.patch
Hi Patrick! I've done additional steps. Am I right?