1334989 - ActorsParent.cpp : Parameters are named differently in declaration vs implementation in AutoSavepoint::Start

Status: RESOLVED FIXED

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

Description

[Andi [:andi]]

A good first bug for a beginner:
here: 

https://dxr.mozilla.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp?q=%2Bfunction%3A%22mozilla%3A%3Adom%3A%3AindexedDB%3A%3A%28anonymous+namespace%29%3A%3ADatabaseConnection%3A%3AAutoSavepoint%3A%3AStart%28const+mozilla%3A%3Adom%3A%3AindexedDB%3A%3A%28anonymous+namespace%29%3A%3ATransactionBase+%2A%29%22&redirect_type=single#11348

The parameter name from the declaration differs from the parameter name from the definition.

Comment 1

[dwivediaakar]

(In reply to Andi-Bogdan Postelnicu from comment #0)
> A good first bug for a beginner:
> here: 
> 
> https://dxr.mozilla.org/mozilla-central/source/dom/indexedDB/ActorsParent.
> cpp?q=%2Bfunction%3A%22mozilla%3A%3Adom%3A%3AindexedDB%3A%3A%28anonymous+name
> space%29%3A%3ADatabaseConnection%3A%3AAutoSavepoint%3A%3AStart%28const+mozill
> a%3A%3Adom%3A%3AindexedDB%3A%3A%28anonymous+namespace%29%3A%3ATransactionBase
> +%2A%29%22&redirect_type=single#11348
> 
> The parameter name from the declaration differs from the parameter name from
> the definition.

Hi,
I modified the code, and the build was successful on my machine.
Is this much enough to check the code.
If yes, then How do I commit the changes to mozilla code base?
If no, then kindly tell me the next steps.

Comment 2

[Andi [:andi]]

Hello,
Great to hear this. You need to attach your patch to this bug and ask for a review from the module owner. A list with the module owners can be found here:

https://wiki.mozilla.org/Modules/All

If the patch is accepted then you need to mark it as checkin-needed.

Comment 3

[dwivediaakar]

Attachment: ActorsParent.cpp

I have changed the definition of the function 
``` nsresult
DatabaseConnection::
AutoSavepoint::Start(const TransactionBase* aTransaction) ```
to
```nsresult
DatabaseConnection::
AutoSavepoint::Start(const TransactionBase* aConnection)```

and changed the name inside the function definition block too.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8831736 [details] [diff] [review]
ActorsParent.cpp

Several things:
* please provide a diff
* please ask only to a single person, don't need more people for a simple patch
* checkin is not needed
* don't need to needinfo on top of review

Comment 5

[dwivediaakar]

(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Comment on attachment 8831736 [details] [diff] [review]
> ActorsParent.cpp
> 
> Several things:
> * please provide a diff
> * please ask only to a single person, don't need more people for a simple
> patch
> * checkin is not needed
> * don't need to needinfo on top of review

Thank you for the suggestions.

Do i need to submit the patch again?
and
How do i provide a diff?

Comment 6

[dwivediaakar]

Attachment: my.diff

Function definition changed.

Comment 7

[Jan Varga [:janv]]

Comment on attachment 8831751 [details] [diff] [review]
my.diff

Review of attachment 8831751 [details] [diff] [review]:
-----------------------------------------------------------------

I'm sorry, but I think we shouldn't change the definition like this. aTransaction makes perfect sense, so change aConnection to aTransaction in the declaration instead.
Thanks.

Comment 8

[dwivediaakar]

Attachment: my.diff

Function declaration changed.

Comment 9

[Jan Varga [:janv]]

Comment on attachment 8831990 [details] [diff] [review]
my.diff

Review of attachment 8831990 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 10

[dwivediaakar]

Comment on attachment 8831990 [details] [diff] [review]
my.diff

Check-in needed.

Comment 11

[Andi [:andi]]

checkin-needed Must be added to keywords field.

Comment 12

[dwivediaakar]

(In reply to Andi-Bogdan Postelnicu from comment #11)
> checkin-needed Must be added to keywords field.

ohkk
So, only mentors and Module owners can change the keywords right?

Comment 13

[Andi [:andi]]

I think you can change keywords as well.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please attach a patch that includes proper commit information when requesting checkin.
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 15

[dwivediaakar]

Attachment: Parameter in the declaration of the function made same as the parameter in definition

Comment 16

[Jan Varga [:janv]]

Comment on attachment 8833604 [details] [diff] [review]
Parameter in the declaration of the function made same as the parameter in definition

Review of attachment 8833604 [details] [diff] [review]:
-----------------------------------------------------------------

You need to add reviewer's name or nick, to the commit message, "; r=janv" in this case.
So it should be:
Bug 1334989 - Parameter in the declaration of the function made same as the one in definition of the function; r=janv

Comment 17

[dwivediaakar]

Attachment: Parameter in the declaration of the function made same as the parameter in definition

Reviewer added in the commit message.

Comment 18

[Jan Varga [:janv]]

Comment on attachment 8833605 [details] [diff] [review]
Parameter in the declaration of the function made same as the parameter in definition

Review of attachment 8833605 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 19

[dwivediaakar]

Comment on attachment 8833605 [details] [diff] [review]
Parameter in the declaration of the function made same as the parameter in definition

checkin-needed 
keyword added

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f39d89313f10
Parameter in the declaration of the function made same as the one in definition of the function; r=janv

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f39d89313f10