Skip to content

Conversation

@ZviBaratz
Copy link
Contributor

I was trying to get a little familiar with the code and thought I would use the opportunity to propose completed docstrings for the parser module. If this sort of contribution is welcome, I could try and do the same for the other modules.

@welcome
Copy link

welcome bot commented Jan 12, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@chrisjsewell
Copy link
Member

always welcome 😄

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

yeh just I think the types should not be duplicated in the docstring (plus they can be properly tested by mypy with the type annotations)

@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #61 (d3d835f) into main (5b13190) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #61   +/-   ##
=======================================
  Coverage   90.75%   90.75%           
=======================================
  Files           6        6           
  Lines         660      660           
=======================================
  Hits          599      599           
  Misses         61       61           
Flag Coverage Δ
pytests 90.75% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sphinx_external_toc/parsing.py 92.09% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b13190...d3d835f. Read the comment docs.

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Nice one!

@chrisjsewell chrisjsewell changed the title [DOC] Updated docstrings. 🔧 MAINTAIN: Updated parser docstrings Jan 12, 2022
@chrisjsewell chrisjsewell merged commit 85f7ee8 into executablebooks:main Jan 12, 2022
@welcome
Copy link

welcome bot commented Jan 12, 2022

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@ZviBaratz ZviBaratz deleted the docstrings branch January 12, 2022 14:42
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