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

Make constants constant, fix broken ref in docs #59

Merged
merged 1 commit into from
Nov 24, 2014
Merged

Make constants constant, fix broken ref in docs #59

merged 1 commit into from
Nov 24, 2014

Conversation

Ichimonji10
Copy link
Contributor

Use tuples instead of lists in module fauxfactory.constants. The built-in
tests function correctly:

$ make test
python -m unittest discover --start-directory tests --top-level-directory .
.................................................................................................................................................................
----------------------------------------------------------------------
Ran 161 tests in 0.934s

OK

Make some changes to documentation:

  • Drop some comments from fauxfactory.constants. An explanation of lorem ipsum
    seems unnecessary, for example.
  • Move remaining descriptions in to the module-wide docstring. This allows
    descriptions to be found both by Sphinx and the help built-in. Also include
    the fauxfactory.constants module in the API documentation. These changes
    allow documentation to be built with zero warnings.

:mod:`fauxfactory.constants`
----------------------------

.. automodule:: fauxfactory.constants
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The :members: argument does not need to be specified, because there are no functions or classes in this module.

Copy link
Owner

Choose a reason for hiding this comment

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

Are you going to remove it then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@omaciel, say again? I don't understand. The :members: argument is not present.

Copy link
Owner

Choose a reason for hiding this comment

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

Guess I don't understand your initial comment... I see a :members: entry on line 10 and you said that it doesn't have to be present...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha! :members: does not need to be an argument to line 15, as fauxfactory.constants contains no functions or classes. :members: should be present on line 10, as fauxfactory does contain functions and/or classes.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the info :)

Use tuples instead of lists in module `fauxfactory.constants`. The built-in
tests function correctly:

    $ make test
    python -m unittest discover --start-directory tests --top-level-directory .
    .................................................................................................................................................................
    ----------------------------------------------------------------------
    Ran 161 tests in 0.934s

    OK

Make some changes to documentation:

* Drop some comments from `fauxfactory.constants`. An explanation of lorem ipsum
  seems unnecessary, for example.
* Move remaining descriptions in to the module-wide docstring. This allows
  descriptions to be found both by Sphinx and the `help` built-in. Also include
  the `fauxfactory.constants` module in the API documentation. These changes
  allow documentation to be built with zero warnings.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling 6585018 on Ichimonji10:constants into 73ad3e0 on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling 6585018 on Ichimonji10:constants into 73ad3e0 on omaciel:master.

@omaciel
Copy link
Owner

omaciel commented Nov 24, 2014

ACK

omaciel added a commit that referenced this pull request Nov 24, 2014
Make constants constant, fix broken ref in docs
@omaciel omaciel merged commit ebde1ef into omaciel:master Nov 24, 2014
@Ichimonji10 Ichimonji10 deleted the constants branch November 24, 2014 22:39
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.

None yet

3 participants