Skip to content
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.

Sign up for GitHub

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

Jump to bottom

HOTFIX: Achieve 100% doc coverage #1387

Merged
merged 1 commit into from Mar 13, 2014
Merged

HOTFIX: Achieve 100% doc coverage #1387

merged 1 commit into from Mar 13, 2014

Conversation

yelled3
Copy link

@yelled3 yelled3 commented Mar 13, 2014

fixing some YARD formatting issues + pull from master

yard stats --list-undoc 

Files:          32
Modules:        19 (    0 undocumented)
Classes:        26 (    0 undocumented)
Constants:       3 (    0 undocumented)
Methods:       145 (    0 undocumented)
 100.00% documented

@yelled3
Copy link
Author

yelled3 commented Mar 13, 2014

@JonRowe @myronmarston

myronmarston
myronmarston reviewed Mar 13, 2014
View reviewed changes
lib/rspec/core/formatters/base_formatter.rb
@@ -71,7 +71,7 @@ def example_group_started(notification)
#
# Invoked at the beginning of the execution of each example.
#
# @param notification [ExampleNotification] containing example subclass of `RSpec::Core::Example`
# @param [ExampleNotification] notification containing example subclass of `RSpec::Core::Example`
Copy link
Member

Choose a reason for hiding this comment

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

Ultimately, I just care about how this renders when you run a yard server (e.g. yard server --reload). Did you confirm that it renders wrongly before and correctly with this change?

Copy link
Author

Choose a reason for hiding this comment

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

screen shot 2014-03-13 at 6 03 57 pm

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Author

Choose a reason for hiding this comment

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

weird thing is - this still renders correctly if you do it the other way (like @JonRowe did)

Copy link
Member

Choose a reason for hiding this comment

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

Funny. I think I actually prefer the @param <name> [<Type>] <description> form because with this way, the name and description run together and it's non-obvious the first word is the param name.

That said, yard issues warnings for that and I care more about being warning free, so changing it like you have is the right thing to do :).

@myronmarston
Copy link
Member

Thanks for following up on this, @yelled3!

myronmarston added a commit that referenced this pull request Mar 13, 2014
@myronmarston
Merge pull request #1387 from yelled3/achieve_100_doc_coverage_hotfix
3aa0a4d 
HOTFIX: Achieve 100% doc coverage
@myronmarston myronmarston merged commit 3aa0a4d into rspec:master Mar 13, 2014
@myronmarston
Copy link
Member

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yelled3 @myronmarston