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

[ci] check JavaScript code with biome tool #6711

Merged
merged 3 commits into from
Nov 3, 2024
Merged

[ci] check JavaScript code with biome tool #6711

merged 3 commits into from
Nov 3, 2024

Conversation

StrikerRUS
Copy link
Collaborator

@StrikerRUS StrikerRUS commented Nov 1, 2024

biome project page: https://biomejs.dev/.

I found it during reading the following article: https://blog.logrocket.com/eslint-adoption-guide/.

The most popular ESLint was too hard to configure (especially after the recent breaking changes: eslint/eslint#18391).

Over the years, many popular and widespread shared configurations have been developed. However, with every breaking change in ESLint, the community projects need to migrate.

Some other alternatives like standard, neostandard and jslint were too aggressive with their feature of absence of configuration.

So I think biome as all-in-one tool (linter+formatter) outside of nodejs environment with easy configuration for non-frontenders is a good choice for LightGBM project.

indent_size = 4
line_length = 120
max_line_length = 120
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines -16 to -18
# Placeholder files
[{*.gitkeep,__init__.py}]
insert_final_newline = none
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, biome fails to read the config here. Also, insert_final_newline=unset|off doesn't help.

configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × Failed to parse the .editorconfig file.
    
    Caused by:
      Custom("expected 'true' or 'false'")

Anyway, none is not allowed value in editorconfig, so these lines haven't actually worked.

echo "Linting Python code"
bash ./.ci/lint-python.sh || exit 1
echo "Linting Python and bash code"
bash ./.ci/lint-python-bash.sh || exit 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to recently added shellcheck in pre-commit hook.

@@ -0,0 +1,21 @@
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@StrikerRUS StrikerRUS changed the title [WIP][ci] check JavaScript code with biome tool [ci] check JavaScript code with biome tool Nov 3, 2024
@StrikerRUS StrikerRUS marked this pull request as ready for review November 3, 2024 02:12
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Interesting! I'm so used to prettier in javascript projects, I'd never really tried anything else. biome looks interesting, and I'm glad we can install it from conda-forge.

I'm open to trying this, even though we only have about 70 lines of Javacsript in this repo... seems cheap to install and like it runs quickly. Thanks!

.editorconfig Outdated Show resolved Hide resolved
@StrikerRUS
Copy link
Collaborator Author

StrikerRUS commented Nov 3, 2024

@jameslamb

and I'm glad we can install it from conda-forge.

Yeah! I was really excited that we can avoid bringing nodejs ecosystem here for just one js-file (and some json-files).

Co-authored-by: James Lamb <[email protected]>
@StrikerRUS StrikerRUS merged commit 13f2e92 into master Nov 3, 2024
48 checks passed
@StrikerRUS StrikerRUS deleted the ci/biome branch November 3, 2024 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants