Skip to content

feat: support Example section for google docstring - #60

Closed
yewentao256 wants to merge 6 commits into
rr-:masterfrom
yewentao256:master
Closed

yewentao256 wants to merge 6 commits into
rr-:masterfrom
yewentao256:master

Conversation

@yewentao256

Copy link
Copy Markdown

No description provided.

@rr- rr- left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for the pull request.

  • Let's rename DocstringExamples to DocstringExample
  • Let's add missing tests
  • Please run black
  • Please change var # type hint to a var: TypeHint

@yewentao256

Copy link
Copy Markdown
Author

Thank you for the reply, I have submitted it again.
If there are still any questions, please let me know.

@yewentao256
yewentao256 requested a review from rr- November 7, 2021 03:45

@rr- rr- left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the changes. A couple of comments left:

Let's remove the assertions on docstring.meta.
Let's change the assertions on test_empty_example from docstring.meta to docstring.examples.

Comment thread docstring_parser/common.py Outdated
return None

@property
def examples(self) -> T.Optional[DocstringDeprecated]:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

T.List[DocstringExample]

@yewentao256

Copy link
Copy Markdown
Author

Thanks for the reply, I've changed it.
If there are still any questions, please let me know.

@yewentao256
yewentao256 requested a review from rr- November 8, 2021 02:18
@rr-

rr- commented Nov 8, 2021

Copy link
Copy Markdown
Owner

It's still T.Optional, whereas the function returns a list :) Please update it to T.List and we should be good to go.

@rr- rr- left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

^

@yewentao256

Copy link
Copy Markdown
Author

Oh I found it, thanks for figuring it out, it should be OK now.

@yewentao256
yewentao256 requested a review from rr- November 9, 2021 04:03
@rr-

rr- commented Nov 9, 2021

Copy link
Copy Markdown
Owner

Merged manually. Thanks.

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