1099094 - Add documentation for session_id in start_session method

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The session_id argument was recently added to start_session but it's not documented. See http://hg.mozilla.org/mozilla-central/file/64206634959a/testing/marionette/client/marionette/marionette.py#l822

On IRC David Burns (AutomatedTester) suggested: "unique identifier for the session. If no session id is passed in then one will be generated by the marionette-server"

The documentation is published here: https://marionette-client.readthedocs.org/en/latest/#marionette.Marionette.start_session

Comment 1

[David Burns :automatedtester]

To get everything setup for Marionette you will need to follow https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

Comment 2

[Ankit Goyal]

I would like to work on it.

Comment 3

[Abhilash Panigrahi]

Attachment: rev1 - Adds documentation for session_id in start_session method

I have attached a patch. It adds the comment for sesseion_id as suggested by David Burns.

Comment 4

[David Burns :automatedtester]

Comment on attachment 8541953 [details] [diff] [review]
rev1 - Adds documentation for session_id in start_session method

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

2 minor nits but otherwise perfect. If you could update the patch with those changes I will land it ASAP! Thanks for your help!

(Just setting it to r- until your new patch comes)

::: testing/marionette/client/marionette/marionette.py
@@ +809,5 @@
>  
>          :param desired_capabilities: An optional dict of desired
>              capabilities.  This is currently ignored.
>          :param timeout: Timeout in seconds for the server to be ready.
> +        :param session_id: unique identifier for the session. If no session id is 

could you remove the extra space after `is`

@@ +810,5 @@
>          :param desired_capabilities: An optional dict of desired
>              capabilities.  This is currently ignored.
>          :param timeout: Timeout in seconds for the server to be ready.
> +        :param session_id: unique identifier for the session. If no session id is 
> +            passed in then one will be generated by the marionette-server

could you remove the hyphen between marionette and server please

Comment 5

[Abhilash Panigrahi]

Attachment: rev2 - Adds documentation for session_id in start_session method

Thank you David! I have made the required changes! Also could you explain what "setting it to -r" means? I am new and don't really understand. :(

Comment 6

[David Burns :automatedtester]

(In reply to Abhilash Panigrahi from comment #5)
> Created attachment 8544009 [details] [diff] [review]
> rev2 - Adds documentation for session_id in start_session method
> 
> Thank you David! I have made the required changes! Also could you explain
> what "setting it to -r" means? I am new and don't really understand. :(

When we land something it needs r+ (review approved) but my initial review was r- (review denied) for a few nits (minor issues). I have now set it to r+ now and will get one of the sheriffs to land it! Great work!

Comment 7

[Abhilash Panigrahi]

(In reply to David Burns :automatedtester from comment #6)
> (In reply to Abhilash Panigrahi from comment #5)
> > Created attachment 8544009 [details] [diff] [review]
> > rev2 - Adds documentation for session_id in start_session method
> > 
> > Thank you David! I have made the required changes! Also could you explain
> > what "setting it to -r" means? I am new and don't really understand. :(
> 
> When we land something it needs r+ (review approved) but my initial review
> was r- (review denied) for a few nits (minor issues). I have now set it to
> r+ now and will get one of the sheriffs to land it! Great work!

Thanks a lot David! I'm glad I could help. I'm excited about contributing more to the community!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8541953 [details] [diff] [review]
rev1 - Adds documentation for session_id in start_session method

Please mark old patches as obsolete when attaching newer revisions :)

Comment 9

[Abhilash Panigrahi]

I'm sorry I missed the checkbox! Will keep that in mind in future. Thanks :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d894b312beb2

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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