Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nodejs] add pollTimeout argument to wait() in WebDriver class #6520

Merged
merged 2 commits into from
Nov 6, 2018
Merged

[nodejs] add pollTimeout argument to wait() in WebDriver class #6520

merged 2 commits into from
Nov 6, 2018

Conversation

CrispusDH
Copy link
Contributor

@CrispusDH CrispusDH commented Oct 10, 2018

Currently, the poll timeout is constant and equal
to 0. In this way polling is as fast as it can.
Also, you can not customize this time.

In java this parameter is argument and by default
is 200 ms.


This change is Reviewable

@CrispusDH CrispusDH changed the title add pollTimeout argument to wait() in WebDriver class [nodejs] add pollTimeout argument to wait() in WebDriver class Oct 15, 2018
@CrispusDH
Copy link
Contributor Author

Could someone give feedback? :) I'm newer in contributing.

@luke-hill
Copy link
Contributor

Best bet is to get a review from someone like @jleyba or maybe @manoj9788 ?

@CrispusDH
Copy link
Contributor Author

Seems nodejs binding does not have any active contributor even for reviewing. Maybe review can be done by guys from another bindings?

@manoj9788
Copy link
Member

manoj9788 commented Oct 30, 2018

@CrispusDH Apologies, for the delay in getting back on this and thank you for your contribution.
In the JsDoc can you please mention that the timeout is specified in milliseconds. Also please add an entry to the CHANGES.md

Currently, the poll timeout is constant and equal
to 0. In this way polling is as fast as it can.
Also, you can not customize this time.

In java this parameter is argument and by default
is 200 ms.
* mention in JsDoc that the timeout is specified in milliseconds
* add an entry to the CHANGES.md
@CrispusDH
Copy link
Contributor Author

@manoj9788 could you take a look at last changes?

@CrispusDH
Copy link
Contributor Author

@manoj9788 Could you tell me what further steps I should do? :) It's my first PR and that's why I don't know what should be done for merging :)

@manoj9788
Copy link
Member

LGTM
ping @jleyba to take look and merge.

@jleyba jleyba merged commit ed3aa1e into SeleniumHQ:master Nov 6, 2018
@CrispusDH CrispusDH deleted the feature/add-poll-timeout-to-wait branch November 6, 2018 20:16
@manoj9788
Copy link
Member

Thanks @jleyba and @CrispusDH

shs96c pushed a commit to shs96c/selenium that referenced this pull request Feb 18, 2019
…iumHQ#6520)

Currently, the poll timeout is constant and equal
to 0. In this way polling is as fast as it can.
Also, you can not customize this time.

In java this parameter is argument and by default
is 200 ms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants