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

Tests docs check script #368

Merged
merged 5 commits into from
Oct 28, 2018
Merged

Tests docs check script #368

merged 5 commits into from
Oct 28, 2018

Conversation

pnevyk
Copy link
Contributor

@pnevyk pnevyk commented Oct 27, 2018

This PR adds a script that checks if there are tests and docs for each rule and they are included in their corresponding "index" files. It was discussed in #366. What exactly is checked is documented in the header of the script. I added this check to pre-commit hook.

Thanks to this script I discovered that three rules have no assertions in readme and that for one rule tests were not executed. I fixed these issues.

Side note: The new post-commit hook crashes when the current branch is not in .git/config yet, which I think is before the push to a remote server. This crash comes from gitdown and transitively from gitinfo. Also in contributing guide there is a sentence that the documentation is built using CI but it seems that it is not true anymore (imo it was removed in this commit). I think that generating docs in CI is better approach than in post commit hook but that's just my hint.

@gajus
Copy link
Owner

gajus commented Oct 27, 2018

Looks great. Thank you.

Is it ready to be merged?

@pnevyk
Copy link
Contributor Author

pnevyk commented Oct 27, 2018

If you like it then I think that yes. Maybe the script can be added to travis script but that's up to you. And if you can in some time resolve the side note I wrote in the description of this PR, it would be great.

@gajus gajus merged commit 318f482 into gajus:master Oct 28, 2018
@gajus
Copy link
Owner

gajus commented Oct 28, 2018

🎉 This PR is included in version 3.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Oct 28, 2018
gajus added a commit that referenced this pull request Oct 28, 2018
@gajus
Copy link
Owner

gajus commented Oct 28, 2018

Side note: The new post-commit hook crashes when the current branch is not in .git/config yet, which I think is before the push to a remote server. This crash comes from gitdown and transitively from gitinfo.

This is a known issue gajus/gitinfo#17

Also in contributing guide there is a sentence that the documentation is built using CI but it seems that it is not true anymore (imo it was removed in this commit). I think that generating docs in CI is better approach than in post commit hook but that's just my hint.

The same issue is preventing generating docs in Travis.

https://travis-ci.org/gajus/eslint-plugin-flowtype/jobs/447405101

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.

None yet

2 participants