Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

(PDOC-161) Add summary tag for short descriptions #138

Merged
merged 3 commits into from
Mar 16, 2017

Conversation

whopper
Copy link
Contributor

@whopper whopper commented Mar 15, 2017

This tag is primarily meant to be applied to Puppet classes,
but it is also supported by every other construct that can be
documented with strings.

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.

I assume there is nowhere we could centralize that warning message? Like a superclass or something?

Same thing with the HTML stuff...


# Warn if a summary longer than 140 characters was provided
if object.has_tag?(:summary) && object.tag(:summary).text.length > 140
log.warn "The length of the summary for function '#{object.name}' exceeds the recommended limit of 140 characters."
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we distinguish between Puppet functions and Ruby functions in the warning?

@whopper
Copy link
Contributor Author

whopper commented Mar 15, 2017

@scotje I toyed with the idea of a new class on top of the Ruby and Puppet handlers to deal with that error message but I thought it ended up being more gross. I figured I'd just follow the convention that already exists for adding a public api tag in each of the handlers. I could look into simplifying it again if you want.

As for the HTML, I'm not sure that there's a better way to do it.

@scotje
Copy link
Contributor

scotje commented Mar 15, 2017

Is there somewhere we could put like a class method just so we have the logic and message all in one place, even if we still have to invoke it from each handler? (With an argument for the context it was invoked in so you can construct a specific message still.)

@whopper
Copy link
Contributor Author

whopper commented Mar 15, 2017

@scotje sure, that seems reasonable. I'll look into it.

@jpadams
Copy link

jpadams commented Mar 16, 2017

I built the gem from @whopper 's branch and it worked just as expected. +1 to merge.

@whopper whopper force-pushed the PDOC-161/master/summary branch from de557d6 to 23ebe11 Compare March 16, 2017 19:18
whopper added 2 commits March 16, 2017 12:51
This tag is primarily meant to be applied to Puppet classes,
but it is also supported by every other construct that can be
documented with strings.
@whopper whopper force-pushed the PDOC-161/master/summary branch from 23ebe11 to 8178c0b Compare March 16, 2017 19:52
module PuppetStrings::Yard::Handlers::Helpers
# Logs a warning if a summary tag has more than 140 characters
def self.validate_summary_tag(object)
if object.has_tag?(:summary) && object.tag(:summary).text.length > 140
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scotje I stuck this in this Helper module for the Handlers. Does that seem reasonable?

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.

Looks good to me!

@whopper whopper force-pushed the PDOC-161/master/summary branch from b831384 to a3f00d0 Compare March 16, 2017 21:22
@scotje scotje merged commit ba1a653 into puppetlabs:master Mar 16, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants