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

Bump ESLint dependencies #1852

Merged
merged 3 commits into from
Mar 25, 2022
Merged

Bump ESLint dependencies #1852

merged 3 commits into from
Mar 25, 2022

Conversation

ryoarmanda
Copy link
Contributor

@ryoarmanda ryoarmanda commented Mar 24, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain: Dependency update

In preparation on adding eslint-config-airbnb-typescript (the Typescript variant of the Airbnb style guide) for #1421, bumped some dependencies regarding to ESLint and other related dependencies.

Overview of changes:
Bumped eslint, eslint-plugin-import, and eslint-config-airbnb-base to the latest version.

  • For the case of eslint, it is bumped to the highest v7 version (though v8 is already available).
  • For the case of eslint-config-airbnb-base, the version after the current one jumps to 15.0.0

Anything you'd like to highlight / discuss:

The eslint-config-airbnb-base bump reports new lint errors to our current codebase, particularly due to some rules that were not enabled before and are enabled now:

  • prefer-regex-literals (~11 new errors)
  • default-param-last (~2 new errors)
  • function-call-argument-newline (most new errors are from this rule, ~29)

I turned all of them off now, but we can discuss below whether we can enable some of the rules and fix some of the violations in this PR, in a separate PR, or keep them disabled.

My opinions:

  • prefer-regex-literals: Should enable, fix the violations.
  • default-param-last: Might enable it, but fixing the violation is a little work (the offender is the Site constructor). Can leave it off too.
  • function-call-argument-newline: Keep it turned off. There's too much cases of both newline and non-newline function calls in our code. Enabling it for one option makes the other case look worse stylistically.

Testing instructions:
Check npm run lint, should be okay.

Proposed commit message: (wrap lines at 72 characters)
Bump ESLint dependencies


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I turned all of them off now, but we can discuss below whether we can enable some of the rules and fix some of the violations in this PR, in a separate PR, or keep them disabled.

I think we can fix the regex one as well and keep the rest off for now. Doing it in another PR is fine. Just need to open an issue for this so we don't forget about it :)

@jonahtanjz jonahtanjz added this to the 4.0 milestone Mar 24, 2022
@jonahtanjz jonahtanjz merged commit 3dac5e8 into MarkBind:master Mar 25, 2022
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

2 participants