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

Add configurations for linting scripts and add global ignores #5843

Conversation

ChunkyProgrammer
Copy link
Member

Add configurations for linting scripts and add global ignores

Pull Request Type

  • Other

Related issue

It seems like the default recommended javascript config was being applied to our scripts files and this was causing issues with lefthook trying to lint the files

Description

I added a specific config for our _scripts folder so we can still use eslint for those files. I also moved some of the ignores to the top of the script which serves as a global ignores (it will apply to all rule below)

Screenshots

Before:

  • add console.log to a file in _scripts (ex: getShakaLocales.js)
    image
  • try to commit:
    image
    After:
  • commit successful

Desktop

  • OS: Linux Mint
  • OS Version: 22
  • FreeTube version: latest nightly

@github-actions github-actions bot added PR: dependencies Pull requests that update a dependency file PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 9, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) October 9, 2024 02:04
@absidue
Copy link
Member

absidue commented Oct 9, 2024

Could you please optimise the ignore patterns according to the docs: https://eslint.org/docs/latest/use/configure/ignore

  • When ignoring directories it is best to add a / to the end of the pattern so that ESLint knows that it is a folder and can skip scanning the directory entirely.
  • The dist, _scripts, .github and node_modules directories only exist in the same directory as the eslint.config.mjs file, so we don't need recusive patterns for those.

You can test the performance of the patterns by calling ESLint without passing any files on the command line, that way it gathers the files to check based on the config file: yarn eslint or npx eslint

@absidue
Copy link
Member

absidue commented Oct 9, 2024

Sorry for only commenting this now, but I wanted to test it on my computer before I commented. The eslint docs mention that **/node_modules/ is on the default ignores list so we don't have to add it ourselves.

[...]. This pattern is added after the default patterns, which are ["**/node_modules/", ".git/"].

absidue
absidue previously approved these changes Oct 9, 2024
eslint.config.mjs Outdated Show resolved Hide resolved
eslint.config.mjs Outdated Show resolved Hide resolved
eslint.config.mjs Outdated Show resolved Hide resolved
@FreeTubeBot FreeTubeBot merged commit 64d73f4 into FreeTubeApp:development Oct 10, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 10, 2024
@ChunkyProgrammer ChunkyProgrammer deleted the eslint-add-global-ignores-lint-scripts branch January 20, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants