-
Notifications
You must be signed in to change notification settings - Fork 16
Inital commit for the auto generation of the api docs #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, thanks @jrbenny35! Note that if you push this to a branch on the mozilla fork then we can get readthedocs to publish them, which could help with reviews.
@@ -0,0 +1,36 @@ | |||
Developer Interface for FoxPuppet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the FoxPuppet documentation, so we don't need 'for FoxPuppet'.
|
||
|
||
Window Manager | ||
============================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The underline should not extend past the heading
.. py:module:: foxpuppet.windows.browser | ||
|
||
.. autoclass:: BrowserWindow | ||
:inherited-members: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structure should be a heading for windows and then a section for each window implementation. We shouldn't need to document the abstract base class.
# Windows
## Window Manager
## Browser Window
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we have to say anything about the 'Windows' directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, users of a Python package will not think of this as a directory, but as a package/module.
:inherited-members: | ||
|
||
|
||
Browser Model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove 'Model' from the headings.
Class that sets up the interface for interacting with the Firefox | ||
browser. | ||
|
||
:param selenium: A selenium object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use intersphinx and link the param type to the official Selenium documentation as we do with PyPOM: https://github.com/mozilla/PyPOM/blob/master/docs/conf.py#L41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good example of this within a docstring? It would work within an rst file but not within a py file.
@@ -37,6 +41,8 @@ def open_window(self, private=False): | |||
|
|||
:param private: Optional parameter to open a private browsing window. | |||
Defaults to False. | |||
|
|||
:returns: A BrowserWindow object of the opened window. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include the return types, linking to the relevant documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking to what specifically?
@@ -5,14 +5,23 @@ | |||
|
|||
class WindowManager(object): | |||
|
|||
""" | |||
A window manager that controls the creation of window objects | |||
for interaction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this description is particularly useful. Perhaps say that it provides access to all open windows. The user doesn't need to know that it creates the window objects.
:returns: A list of BrowserWindow Instances | ||
Sets all current windows to appropriate window instances. | ||
|
||
:returns: A list of Window Instances |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include the return types.
@davehunt Thanks for the review. When you say push to a branch on the mozilla fork, you mean create a branch on the Mozilla fork of foxpuppet? |
I pushed to a branch here but the docs aren't rendering the new changes. I can do a build locally and get them however. |
I've enabled the sphix_auto_docs branch as a protected version here: http://foxpuppet.readthedocs.io/en/sphinx_auto_docs/api.html. As you can see the autogenerated docs are not showing up yet. Please close this pull, push only to the branch on the mozilla fork for now, and when you're ready open a new pull request from the new branch. |
Closing. |
Just a rough draft really.
@davehunt r?
Also after this I will do the User Guide.