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

Apply linting rules. #238

Merged
merged 5 commits into from
Nov 7, 2022
Merged

Apply linting rules. #238

merged 5 commits into from
Nov 7, 2022

Conversation

jgerigmeyer
Copy link
Member

This applies the existing linting rules, and adds a few rules to match the .editorconfig settings.

Here are a few other guidelines that cannot be enforced via ESLint:

- When you define a function, use a space between the opening paren and its name. That way we can search for "functionName (" and find its definition immediately, instead of having to wade through calls to the function.
- Prefer single-word names over multi-word names. 3+ word names are especially frowned upon.
Copy link
Member

Choose a reason for hiding this comment

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

Why are we removing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@LeaVerou I removed this note because I added an ESLint rule to enforce this convention -- so I thought the note is no longer applicable. If you prefer, I'm happy to add it back.

Copy link
Member

Choose a reason for hiding this comment

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

No need :) Thanks for the explanation!

Copy link
Member

@LeaVerou LeaVerou left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@jgerigmeyer jgerigmeyer merged commit 0b3caca into color-js:main Nov 7, 2022
@jgerigmeyer jgerigmeyer deleted the lint branch November 7, 2022 17:10
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