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

Enable id-denylist and id-length in base config #200

Merged
merged 6 commits into from Sep 8, 2022

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Aug 19, 2021

This modifies the base config to forbid specific identifier names (e.g. err, cb) and variable and function names shorter than two characters (with some exceptions).

@rekmarks rekmarks changed the title RFC: Enable id-denylist and id-length in base config Enable id-denylist and id-length in base config Aug 28, 2021
@rekmarks rekmarks marked this pull request as ready for review August 28, 2021 20:15
@rekmarks rekmarks requested a review from a team as a code owner August 28, 2021 20:15
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Well... this seems like it would have a lot of violations 🤔 Has this been tested on the extension or controllers repo?

packages/base/src/index.js Outdated Show resolved Hide resolved
@rekmarks rekmarks force-pushed the forbid-stupid-identifiers branch 2 times, most recently from 3851d69 to e11ca49 Compare September 24, 2021 17:15
package.json Show resolved Hide resolved
@rekmarks
Copy link
Member Author

@Gudahtt a couple of tests:

@metamask/controllers (not bad at all)
eslint-result-controllers.txt

✖ 62 problems (62 errors, 0 warnings)

metamask-extension (:sweat_smile:, after adding an additional exception for t)
eslint-result-extension.txt

✖ 857 problems (857 errors, 0 warnings)

@mcmire
Copy link
Contributor

mcmire commented Apr 22, 2022

I think this is a good idea. Too bad we can't configure ESLint to autofix these for us.

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

I'm in favor of this. This rule can be disabled in the extension until the violations can be gone through.

@Gudahtt
Copy link
Member

Gudahtt commented Aug 15, 2022

I am in favour of this as well. I can approve once v10 has been published. Better to keep this for the v11 release to avoid making the v10 update too challenging.

@rekmarks rekmarks merged commit 6560a01 into main Sep 8, 2022
@rekmarks rekmarks deleted the forbid-stupid-identifiers branch September 8, 2022 22:58
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.

None yet

3 participants