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

docs: update lefthook example #662

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

Conversation

AkaraChen
Copy link

@AkaraChen AkaraChen commented Jun 24, 2024

Summary

If the root of the lefthook command is not specified, biome check will not follow biome.json[overrides]

@AkaraChen AkaraChen requested a review from a team as a code owner June 24, 2024 04:36
Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 18e2a31
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6678f7e5347538000868729f
😎 Deploy Preview https://deploy-preview-662--biomejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ematipico
Copy link
Member

@nhedger is the change make sense to you? You used lefthook before

@nhedger
Copy link
Member

nhedger commented Jun 24, 2024

I've never used root before, and I'm not sure what the issue is in the first place. I'd like to reproduce the issue to confirm that this is indeed a documentation issue, and not a bug in Biome itself.

Would you be willing to provide a reproduction @AkaraChen ?

@AkaraChen
Copy link
Author

Here's the step by step guide to reproduce:

https://github.com/AkaraChen/biomejs-repro-662

@nhedger

@nhedger
Copy link
Member

nhedger commented Jun 25, 2024

Thanks for the detailed reproduction; this helped a lot!

I can reproduce the issue by following your instructions, but I suspect that the pattern specified in includes does not match the files in dir-1, which would explain why the file still gets linted.

Using the following value as include seems to correctly prevent Biome from linting the files in dir-1: ./dir-1/*.

What I can't yet explain, though, is why setting the root setting in lefthook somehow makes it work.


@ematipico do we allow specifying folders as-is in the includes, or is it mandatory to specify a glob that matches files.

@Sec-ant
Copy link
Member

Sec-ant commented Jun 25, 2024

Here're some relative discussions on this glob match behavior: biomejs/biome#2131 (comment)

@ematipico
Copy link
Member

What I can't yet explain, though, is why setting the root setting in lefthook somehow makes it work.

It's possible that with this setting, lefthook computes the paths differently, which results in matching the glob specified.

However, if there's an issue, it's definitely around globs and not how lefthook should be set up.

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.

4 participants