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

Addition: lowercase-name only for tests #76

Closed
nevir opened this issue Feb 13, 2018 · 10 comments · Fixed by neinteractiveliterature/intercode#704, severest/retrobot#69 or autolist/eslint-config-autolist#2

Comments

@nevir
Copy link

nevir commented Feb 13, 2018

It'd be helpful to allow uppercase names for describes (e.g. subjects - ClassName, etc), while still enforcing lowercase test titles

@macklinu
Copy link
Collaborator

We could provide an option that allows for ignoring the check for describe().

{
  "jest/lowercase-name": ["error", {
    "ignore": ["describe"]
  }]
}

I'm also wondering if lowercase-name should check describe() at all, or if only checking it() and test() is more valuable. 🤔

@nevir
Copy link
Author

nevir commented Feb 13, 2018 via email

@SimenB
Copy link
Member

SimenB commented Feb 13, 2018

I like the idea of a regex (and an ignore array). PR welcome for both!

@macklinu
Copy link
Collaborator

This code introduced in #74 could be a starting point for introducing an ignore option to the lowercase-name rule.

const whitelistedHookNames = (
context.options[0] || { allow: [] }
).allow.reduce((hashMap, value) => {
hashMap[value] = true;
return hashMap;
}, Object.create(null));

@with-heart
Copy link
Contributor

I submitted a PR to add the ignore option for this rule. I started to do some work on regex but it doesn't really feel like the description message regex belongs in the lowercase-name rule. Thoughts?

@nevir
Copy link
Author

nevir commented Feb 27, 2018

maybe rename the rule to name, naming-style or something along those lines?

@SimenB
Copy link
Member

SimenB commented Feb 28, 2018

Renaming it is a breaking change... We have a few breaking changes lined up anyways, so not a blocker. Worth it, though?

@nevir
Copy link
Author

nevir commented Feb 28, 2018

Unsure! could maybe duplicate it into the new name & deprecate old, if you want a smoother transition (but a bit more annoying to maintain)

@SimenB SimenB closed this as completed in 03a7cda Mar 14, 2018
@SimenB
Copy link
Member

SimenB commented Mar 14, 2018

This should be covered by the new option, agreed @nevir?

Might be worth opening up a new issues for renaming the option. My vote is for a new major once node 4 is EOL in April anyways 🙂

@SimenB
Copy link
Member

SimenB commented Mar 15, 2018

🎉 This issue has been resolved in version 21.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment