979657 - [marionette-js-client] The documentation for the findElements function is not correct.

Status: RESOLVED FIXED

(Testing Graveyard :: JSMarionette, defect)

Description

[Evan Tseng [:evanxd]]

It should say that return an Array, not Object.
https://github.com/mozilla-b2g/marionette-js-client/blob/master/lib/marionette/element.js#L82

Comment 1

[Evan Tseng [:evanxd]]

Hi Jing-Kai,

Do you have any problem for this bug?
We could discuss here.

Thanks.

Comment 2

[Evan Tseng [:evanxd]]

Hi Jing-Kai,

After you done the patch,
please send a pull request to http://github.com/mozilla-b2g/gaia
and send me a review request with following the https://speakerdeck.com/eragonj/all-about-firefox-os?slide=68 doc.

Thanks.

Comment 3

[Jing.Kai Lin]

I'm sorry.
I done  the homework of other subject before.

My problem is that "I don't know what should I do for this bug ".

Line 82 :  @return {Object} self.
It seem to modify that to " @return {Array} self. "  .
It's so easy and direct,so I'm not sure.

I don't know what is your  expectation.

Comment 4

[Evan Tseng [:evanxd]]

Hi Jing.Kai,

Got you.
You just need to change the 82 line as `@return {Array} elements matched the query string.`

Then follow Comment 2 to send pull request and review request.
After this, do you already know how to fix this bug?

Thanks.

Comment 5

[Jing.Kai Lin]

Attachment: modify the comment to match the funtion

Comment 6

[Jing.Kai Lin]

Thanks Evan,

I know what I should do.
I already try to send pull request and review request.

Comment 7

[Evan Tseng [:evanxd]]

Hi Jing-Kai,

Nice Job!
And I left a comment for your patch.
Please update for that, then we could r+ and merge it.

Thanks.

Comment 8

[Jing.Kai Lin]

Attachment: update line 82

Sorry , I didn't do right before.

I already update the line82.
Please check.
Thanks.

Comment 9

[Evan Tseng [:evanxd]]

Hi Jing-Kia,

Sorry, my point is that you should update you commit message as `Bug 979657 - Update the documentation for findElements method.`.

And please make sure that only one commit in a pull request.
Your pull request have two commits.

Thanks.

Comment 10

[Jing.Kai Lin]

Hi Evan
I canceled old request,and then pulled new request that has one commit and true message.

And I'm sorry.
I just find out the comment 2 "please send a pull request to http://github.com/mozilla-b2g/gaia".
It means "gaia", not "marionette-js-client".
I don't know what should I pull request to "gaia".

Do review request need two URL,
one to  the pull of "gaia" ,another to the pull of "marionette-js-client" ?

Comment 11

[Evan Tseng [:evanxd]]

Hi Jing-Kai,

I made a mistake.
We should send a pull request with your this patch to https://github.com/mozilla-b2g/marionette-js-client.
Really sorry.

And please make sure that only one commit in a pull request and the commit message is `Bug 979657 - Update the documentation for findElements method.`.

Go, go!
Hope we could finish this bug before the afternoon.

Thanks.

Comment 12

[Evan Tseng [:evanxd]]

Do NOT send to http://github.com/mozilla-b2g/gaia.
It is my bad.

Comment 13

[Evan Tseng [:evanxd]]

Please refer to http://www-cs-students.stanford.edu/~blynn/gitmagic/intl/en/ch05.html to change the commit message.
Thanks.

Comment 14

[Jing.Kai Lin]

Attachment: new pull request

Hi Evan 

Sorry that I'm in class until 16:00 today.
I don't understand how delete commit.
(Where use instruction,and how to choose commit that I want delete)
So I pulled new request .
But I will do my best to learn how to use git in next days.

Comment 15

[Evan Tseng [:evanxd]]

Comment on attachment 8391086 [details] [review]
new pull request

Hi Jing-Kai,

r+, nice shot!

Thanks.

Comment 16

[Evan Tseng [:evanxd]]

master: https://github.com/mozilla-b2g/marionette-js-client/commit/49bdefc573a882fc81403d7cfbf2705ca0e6c067