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

fix: include meta.docs.url for all rules #60

Merged
merged 3 commits into from
Feb 9, 2018

Conversation

macklinu
Copy link
Collaborator

@macklinu macklinu commented Feb 8, 2018

This enables and fixes the eslint-plugin/require-meta-docs-url rule. The diff is large since most of the rules were converted from function(context) to { meta, create } signatures (it's much more manageable to look at when appending ?w=1 to the pull request URL).

One way this PR could be improved:

The require-meta-docs-url rule is fixable, given the following option:

{
  "eslint-plugin/require-meta-docs-url": ["error", {
    "pattern": "https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/{{name}}.md"
  }]
}

The rule will validate all meta.docs.url properties to match the provided pattern, replacing {{name}} with the filename.

The current issue is that the rule filenames are snake-cased (valid_expect) and the doc filenames are kebab-cased (valid-expect), so the require-meta-docs-url rule will generate links that 404. Would it be worthwhile to rename the rule and test files to use kebab-casing, or perhaps rename the documentation files to use snake-casing? I don't know what the preferred convention is in this codebase, and I didn't want to introduce a change like that without pointing it out and discussing first.

(meta: I wasn't sure if the commit prefix should be chore, refactor, or feat, so please feel free to comment with what I should change it to or change it when squashing - thanks 😄 )

@SimenB
Copy link
Member

SimenB commented Feb 8, 2018

Would it be worthwhile to rename the rule and test files to use kebab-casing

Yes please, it is the way it is because that's the way it is in the core Jest repo. Feel free to normalize to kebab-case.

I wasn't sure if the commit prefix should be chore, refactor, or feat

depends on if we want it to be released or not. Will this make any difference too tooling? If yes, I guess fix is fine, otherwise chore

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Rename files so this can be autofixed, like you mentioned

@macklinu
Copy link
Collaborator Author

macklinu commented Feb 8, 2018

Thanks, will push up some new commits to address this, @SimenB 👍

@macklinu macklinu changed the title chore: include meta.docs.url for all rules fix: include meta.docs.url for all rules Feb 8, 2018
@macklinu
Copy link
Collaborator Author

macklinu commented Feb 9, 2018

This is ready for another look.

@SimenB SimenB merged commit 33bdfbf into jest-community:master Feb 9, 2018
@macklinu macklinu deleted the require-meta-docs-url branch February 9, 2018 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants