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.

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

simplify the stats role #8

Closed
wants to merge 2 commits into from
Closed

simplify the stats role #8

wants to merge 2 commits into from

Conversation

@yanick
Copy link
Contributor

@yanick yanick commented May 13, 2018

moved the definitions out of the BEGIN block, which didn't seem to be needed after all.

@olof
Copy link
Owner

@olof olof commented Nov 28, 2019

Like I said in #7, I'm so sorry about not getting back to you on this until now! Overall, this change looks good (I'm still trying to re-familiarize myself with the code), but pod-coverage fails because it's now become clear to it that read_all exists and should be documented. That's not something you need to deal with though.

I'll rename the test file to t/10-stats.t and some other little editorial changes. But really cool that you added tests, appreciated!

@olof olof self-assigned this Nov 28, 2019
@yanick
Copy link
Contributor Author

@yanick yanick commented Nov 28, 2019

Pleasure is all mine! :-)

@olof
Copy link
Owner

@olof olof commented Nov 28, 2019

Landed as 9c97fc9 on master, and is now part of the 0.0806 release. Thanks for your contributions! :)

@olof olof closed this Nov 28, 2019
@yanick
Copy link
Contributor Author

@yanick yanick commented Nov 28, 2019

Awesome. ^.^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.