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

feat: use biome instead of eslint for linting #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LucasB25
Copy link

@LucasB25 LucasB25 commented Aug 28, 2024

  • Remove eslint
  • Add biome
  • Update packages

Fixed the concerns that lint detected

npm run lint

@LucasB25 LucasB25 requested a review from Deivu as a code owner August 28, 2024 16:57
- Remove eslint
- Add biome
- Update packages

- Fixed the concerns that lint detected "npm run lint"
@Deivu Deivu added enhancement New feature or request on hold Still deciding on how this would go labels Aug 28, 2024
@Deivu
Copy link
Member

Deivu commented Aug 28, 2024

We recently moved our configs to a dedicated repo which is here https://github.com/shipgirlproject/eslint-config.

Do biome support this configuration? We are transitioning to a global lint settings

@LucasB25
Copy link
Author

You have to look at the biome site to get the best configuration for the code but yes it is extremely complete

@Deivu
Copy link
Member

Deivu commented Aug 28, 2024

Putting this on hold for the meantime as I'm doing other things, This may get rejected since we are gonna favor global configs, but we'll see

@LucasB25
Copy link
Author

Eslint and bad, biome allows to have a code much cleaner knowing that it signals you the code errors and there are quite a lot of them currently

@0t4u
Copy link
Member

0t4u commented Aug 29, 2024

No. Eslint has a mature ecosystem of plugins and rules, and last time I checked biome is not production ready. Eslint speed is "good enough" for a relatively small project such as shoukaku, and just does not make sense for us.

@0t4u
Copy link
Member

0t4u commented Aug 29, 2024

Eslint and bad, biome allows to have a code much cleaner knowing that it signals you the code errors and there are quite a lot of them currently

Your point makes no sense because what you're describing is just a linter, which both eslint and biome are.

Why is eslint bad?

@0t4u 0t4u changed the title Update feat: use biome instead of eslint for linting Aug 29, 2024
@EvilG-MC
Copy link
Contributor

EvilG-MC commented Aug 29, 2024

The only reason for this is: "I don't like ESLint and I don't want them to use it either." I find it a bit... Strange, let's leave it at that, wanting to change something that in this case, these guys seem to work properly and without problems.

I don't use ESLint but if it works for them, great.

The only thing I can add is to add a pre-commit to format the code in the commits. Maybe I'll do a PR for it.

@0t4u
Copy link
Member

0t4u commented Aug 29, 2024

The only reason for this is: "I don't like ESLint and I don't want them to use it either." I find it a bit... Strange, let's leave it at that, wanting to change something that in this case, these guys seem to work properly and without problems.

I don't use ESLint but if it works for them, great.

The only thing I can add is to add a pre-commit to format the code in the commits. Maybe I'll do a PR for it.

Code style is already enforced by CI and you need those to pass to merge PRs anyways. It should be fine as is.

@Deivu
Copy link
Member

Deivu commented Aug 29, 2024

The only reason for this is: "I don't like ESLint and I don't want them to use it either." I find it a bit... Strange, let's leave it at that, wanting to change something that in this case, these guys seem to work properly and without problems.
I don't use ESLint but if it works for them, great.
The only thing I can add is to add a pre-commit to format the code in the commits. Maybe I'll do a PR for it.

Code style is already enforced by CI and you need those to pass to merge PRs anyways. It should be fine as is.

The thing is, eslint fails to enforce tabs and indents

@0t4u
Copy link
Member

0t4u commented Aug 29, 2024

The only reason for this is: "I don't like ESLint and I don't want them to use it either." I find it a bit... Strange, let's leave it at that, wanting to change something that in this case, these guys seem to work properly and without problems.

I don't use ESLint but if it works for them, great.

The only thing I can add is to add a pre-commit to format the code in the commits. Maybe I'll do a PR for it.

Code style is already enforced by CI and you need those to pass to merge PRs anyways. It should be fine as is.

The thing is, eslint fails to enforce tabs and indents

It is in the rules, and tbh we should probably use prettier for that purpose.

@EvilG-MC
Copy link
Contributor

I see, thanks for the explanation I appreciate it.

@LucasB25
Copy link
Author

LucasB25 commented Aug 29, 2024

The only reason for this is: "I don't like ESLint and I don't want them to use it either." I find it a bit... Strange, let's leave it at that, wanting to change something that in this case, these guys seem to work properly and without problems.

I don't use ESLint but if it works for them, great.

The only thing I can add is to add a pre-commit to format the code in the commits. Maybe I'll do a PR for it.

Since the latest version of ESLint, many plugins no longer work, causing issues that force you to stick with the previous version. Personally, I don't mind, but Biome is much more efficient in every aspect, far less tedious to configure, and it catches a lot more errors and inconsistencies in the code.

do a simple 'npm run lint' and you will see that there are problems in the logic of the code that eslint would not have provided

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request on hold Still deciding on how this would go
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants