Closed Bug 1334986 Opened 9 years ago Closed 5 years ago

ActorsParent.cpp : Parameters are named differently in declaration vs implementation in TransactionBase::VerifyRequestParams

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
89 Branch
Tracking Flags:
Tracking Status
firefox54 --- wontfix
firefox89 --- fixed

People

(Reporter: andi, Assigned: snehasai01, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [lang=C++])

Attachments

(2 files, 1 external file, 2 obsolete files)

renamedParameter1.patch
1.47 KB, patch
Details | Diff | Splinter Review
new_patch.patch
2.53 KB, patch
peterv
: review+
Details | Diff | Splinter Review
Component: Geolocation → DOM
Summary: nsGeolocation.cpp: redundant return statement at the end of a function with a void return type → ActorsParent.cpp : Parameters are named differently in declaration vs implementation in TransactionBase::VerifyRequestParams
can i work on this bug?
Sure go ahead! Thx
Attachment #8832166 - Flags: review?(bpostelnicu)
I'm not a reviewer for this module, please see the module owners: https://wiki.mozilla.org/Modules/All
Attachment #8832166 - Flags: review?(bpostelnicu)
Comment on attachment 8832166 [details] [diff] [review] renamed the parameter in definition as it was in declaration We are getting closer, the patch looks OK, thank you, now you just need to ask for a review from a module owner: https://wiki.mozilla.org/Modules/All As this is part of dom, here are the eligible reviewers: https://wiki.mozilla.org/Modules/All#Document_Object_Model
Attachment #8832166 - Flags: feedback+
sorry i didn't catch you early and yeah i will ask for review from module owner. Thanx :)
Attachment #8832166 - Flags: review?(peterv)
(In reply to Andi-Bogdan Postelnicu from comment #5) > Comment on attachment 8832166 [details] [diff] [review] > renamed the parameter in definition as it was in declaration > > We are getting closer, the patch looks OK, thank you, now you just need to > ask for a review from a module owner: > https://wiki.mozilla.org/Modules/All > > As this is part of dom, here are the eligible reviewers: > https://wiki.mozilla.org/Modules/All#Document_Object_Model Now i have asked the module owner for the review.
Assignee: nobody → amanaryan23
Comment on attachment 8832166 [details] [diff] [review] renamed the parameter in definition as it was in declaration Review of attachment 8832166 [details] [diff] [review]: ----------------------------------------------------------------- Shouldn't you do the same change to the VerifyRequestParams that takes a const OptionalKeyRange&? ::: dom/indexedDB/ActorsParent.cpp @@ +15210,4 @@ > > return true; > } > +// renamed the parameter as defined earlier Don't add this comment.
Attachment #8832166 - Flags: review?(peterv) → review-
Sure. I will make the change.
Made changes for the VerifyRequestParams that takes a const OptionalKeyRange& and also removed the comment as suggested.
Attachment #8833338 - Flags: review?(peterv)
Comment on attachment 8833960 [details] [diff] [review] Bug 1334986 - Rectified function definition of VerifyRequestParams for SerializedKeyRange and OptionalKeyRange There is already someone working on this patch. Sorry
Attachment #8833960 - Attachment is obsolete: true
Attachment #8833960 - Flags: review?(jvarga)
Comment on attachment 8833960 [details] [diff] [review] Bug 1334986 - Rectified function definition of VerifyRequestParams for SerializedKeyRange and OptionalKeyRange Review of attachment 8833960 [details] [diff] [review]: ----------------------------------------------------------------- This looks good.
Attachment #8832166 - Attachment is obsolete: true
Can you please attach a combined patch of all the changes?
Flags: needinfo?(amanaryan23)
Attached patch new_patch.patch — — Splinter Review
I have attached the combined patch of all the changes.
Attachment #8835603 - Flags: review?(peterv)
Attachment #8835603 - Flags: review?(peterv) → review+
Attachment #8833338 - Flags: review?(peterv)
Can someone help me out? I am a new developer and I wanted to try my hands on this bug.How do I get it's source code to?
(In reply to Aman Kumar from comment #15) > Created attachment 8835603 [details] [diff] [review] > new_patch.patch > > I have attached the combined patch of all the changes. Great job, it seems that the patch is now reviewed and can be committed. But before that there is still one more step to do. Can you please change the name of the patch from: >>Bug 1334986 - renamed the parameter in definition as it was in declaration of >>TransactionBase::VerifyRequestParams to >>Bug 1334986 - renamed the parameter in definition as it was in declaration of TransactionBase::VerifyRequestParams. r=peterv In this way we also include the reviewers nick to the commit message. After you do this you only need to update the patch. After the patch is updated it must be marked as checkin-needed in the keywords section. If you need help just let me know!
(In reply to Shashwat Mishra from comment #16) > Can someone help me out? I am a new developer and I wanted to try my hands > on this bug.How do I get it's source code to? Hello, this bug is almost finished, but you can search for other bugs having keywords like: good-first-bug
(In reply to Andi-Bogdan Postelnicu from comment #18) > (In reply to Shashwat Mishra from comment #16) > > Can someone help me out? I am a new developer and I wanted to try my hands > > on this bug.How do I get it's source code to? > > Hello, this bug is almost finished, but you can search for other bugs having > keywords like: good-first-bug Well can you allot me an easy bug to start with?
(In reply to Shashwat Mishra from comment #19) > (In reply to Andi-Bogdan Postelnicu from comment #18) > > (In reply to Shashwat Mishra from comment #16) > > > Can someone help me out? I am a new developer and I wanted to try my hands > > > on this bug.How do I get it's source code to? > > > > Hello, this bug is almost finished, but you can search for other bugs having > > keywords like: good-first-bug > Well can you allot me an easy bug to start with? What do you say about this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1337294 If you are planning on working on this you could simply assign it on your name.
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INACTIVE
Flags: needinfo?(amanaryan23)
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Priority: -- → P5
Component: DOM → DOM: Core & HTML

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: amanaryan23 → nobody
Status: REOPENED → NEW

Since it is reopened, I want to work on this bug.

:andi, are you still available as mentor here? Thank you!

Flags: needinfo?(bpostelnicu)

Sure I can mentor this.
Sachin if you want to work on this please assign it to you.

Flags: needinfo?(bpostelnicu) → needinfo?(sachinthakan001)

(In reply to Andi-Bogdan Postelnicu [:andi] from comment #25)

Sure I can mentor this.
Sachin if you want to work on this please assign it to you.

Hi, I am actually confused about earlier patches. They were looking fine. What was wrong there ?? Thanks

Flags: needinfo?(sachinthakan001) → needinfo?(bpostelnicu)

(In reply to Sachin from comment #26)

(In reply to Andi-Bogdan Postelnicu [:andi] from comment #25)

Sure I can mentor this.
Sachin if you want to work on this please assign it to you.

Hi, I am actually confused about earlier patches. They were looking fine. What was wrong there ?? Thanks

The patch was OK, it wasn't landed. I think the patch needs to be rebased and ask for review one more time.

Flags: needinfo?(bpostelnicu)
Assignee: nobody → snehasai01
Status: NEW → ASSIGNED
Pushed by kgupta@mozilla.staktrace.com: https://hg.mozilla.org/integration/autoland/rev/615d4abfd69e changed the parameters in declarations and definitions of Transaction Base::VerifyRequestParams. r=sg

https://hg.mozilla.org/mozilla-central/rev/615d4abfd69e

Status: ASSIGNED → RESOLVED
Closed: 8 years ago → 5 years ago
status-firefox89: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: