Doctests for Account and other general doc cleanup #94
Conversation
kclowes
changed the title
There was a problem hiding this comment.
LGTM. I'm assuming that you ran the tests locally, and that was part of how you found all the little fixups. Were you able to test that CircleCI is running doctest correctly? (ie~ push a doc that should fail in CI to make sure it turns red)
Since this is the first time turning on doctests, AFAIK, would be nice to have a quick gut-check that everything is wired together.
| @@ -46,7 +46,7 @@ def address(self): | |||
| @property | |||
| def privateKey(self): | |||
| """ | |||
| .. CAUTION:: Deprecated for :var:`~eth_account.signers.local.LocalAccount.key`. | |||
| .. CAUTION:: Deprecated for :meth:`~eth_account.signers.local.LocalAccount.key`. | |||
There was a problem hiding this comment.
Ah interesting, so the link changes to :meth: when it's a @property. Did this show up during doc tests? You were able to confirm that it links correctly with this change?
There was a problem hiding this comment.
Yep, it was showing up as a warning in the doc build. And it does work correctly!
|
@carver I had run the tests using the circleci CLI locally, but not all together, so I'm glad I did that. Good suggestion! It looks like the integration and core tests were running tests in the |
|
I had a vague memory that they were intentional. Luckily it doesn't change too often, so a git blame quickly showed the commit that added it: 8d99dea It does look like it's necessary to leave it in so that doctest runs against any docstring examples. I think the inclusion in the integration test is just copy/pasta though: Although, one option would be to leave the change you did already and then add an extra tox.ini command under the That way any docstring doctest failures would show up in the |
change references to doc to all be docs
What was wrong?
There were no doctests in the eth-account docs.
Issue #9
How was it fixed?
Added some!
Cute Animal Picture