Skip to content

(PDOC-129) Include tags in overload objects when serialized as JSON #122

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

Merged

Conversation

whopper
Copy link
Contributor

@whopper whopper commented Nov 4, 2016

Previously, overload objects were not displaying their tags when
they had no docstring text. This was due to an issue in the overload
to_hash method, which prevented the tags from being serialized when
the dispatch had no top-level text. This commit updates that logic
so that the tags will always be included in the hash if they exist.

@whopper
Copy link
Contributor Author

whopper commented Nov 4, 2016

This should probably be merged after #119 to prevent a bigger rebasing headache. I'll have to rebase this once that's merged, though.

@whopper whopper force-pushed the PDOC-129/master/overload-empty-docstring branch 2 times, most recently from 6eb2e8b to 94c06e7 Compare November 4, 2016 20:21
Copy link
Contributor

@scotje scotje left a comment

Choose a reason for hiding this comment

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

In this case, "docstring" is a class from YARD correct? It seems like it's confusing that it provides an "empty?" method that doesn't consider tags. :/

@whopper
Copy link
Contributor Author

whopper commented Nov 4, 2016

@scotje indeed, it's a bit dumb. Actually, we may be able to use the Docstring.blank? method: http://www.rubydoc.info/gems/yard/YARD/Docstring#blank%3F-instance_method

@scotje
Copy link
Contributor

scotje commented Nov 4, 2016

Yeah, it looks like blank? does what we want in this case?

@whopper whopper force-pushed the PDOC-129/master/overload-empty-docstring branch from 94c06e7 to ceb8156 Compare November 4, 2016 22:37
@whopper
Copy link
Contributor Author

whopper commented Nov 4, 2016

@scotje updated to use blank? and verified it works as expected.

@scotje
Copy link
Contributor

scotje commented Nov 4, 2016

Hooray!

@hlindberg
Copy link
Contributor

LGTM

Previously, overload objects were not displaying their tags when
they had no docstring text. This was due to an issue in the overload
`to_hash` method, which prevented the tags from being serialized when
the dispatch had no top-level text. This commit updates that logic
so that the tags will always be included in the hash if they exist.
@whopper whopper force-pushed the PDOC-129/master/overload-empty-docstring branch from ceb8156 to c56d9c6 Compare November 14, 2016 19:46
@whopper
Copy link
Contributor Author

whopper commented Nov 14, 2016

@scotje rebased!

@scotje scotje merged commit 70d5b1c into puppetlabs:master Nov 14, 2016
@eputnam eputnam added the bugfix label Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants