Skip to content

Conversation

@aaltat
Copy link
Contributor

@aaltat aaltat commented Nov 9, 2017

Part of #882

:param driver: Instance of the Selenium `WebDriver`.
:param alias: Alias given for this `WebDriver` instance.
:type driver: selenium.webdriver.remote.webdriver.WebDriver
:type alias: intger or string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I'd prefer having :type right after the matching :param.
  • Type of alias seems wrong in many ways. Should it be a class, not free text? There's a typo. And alias actually is always a string.
  • Do we need to specify the type for all arguments, including strings? I noticed you hadn't added types to such arguments in other places and I think that's OK.

:param parent: Optional parent `WebElememt` to search child elements
from. By default search starts from the root using `WebDriver`.
:type parent: selenium.webdriver.remote.webelement.WebElement
:return: list of found `WebElement` or `[]` if element not found.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't too clear. Wouldn't just List of found elements. be enough when the type specified explicitly below. If you want to explain the list can be empty (which ought to be pretty obvious), something like If no matching elements are found, the list is empty. could be added as well.

Always returns a list of `WebElement` objects. If no matching element
is found, the list is empty. Otherwise semantics are exactly same
as with :meth:`find_element`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicate information now that arguments are explained above and can be removed.

:param parent: Optional parent `WebElememt` to search child elements
from. By default search starts from the root using `WebDriver`.
:type parent: selenium.webdriver.remote.webelement.WebElement
:return: list of found `WebElement` or `[]` if element not found.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this explanation can be enhanced and the paragraph below containing duplicate information removed.

@aaltat aaltat force-pushed the update_sphix_type_definitions branch from 1e5507b to fb963b0 Compare November 14, 2017 20:18
@aaltat aaltat merged commit 89e45c6 into robotframework:master Nov 14, 2017
@aaltat aaltat deleted the update_sphix_type_definitions branch November 14, 2017 20:40
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.

2 participants