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

fix(cli): remove ignore from crawling, and fix regression #1249

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Dec 18, 2023

Summary

Closes #1247

The issue was introduced a long time ago (before the fork) and it went unnoticed by the majority of the contributors.

This caused some weird issues and strange things from my end. I hunted down the issue and created some tests to avoid regressions.

Unfortunately, now that Biome gained some usage, we can't emit errors, otherwise, it will break many CIs once we release this fix. Because of that, I decided to emit a warning instead.

Also, I decided to remove the ignore crate for crawling the file system. Unfortunately, it doesn't work very well for our needs, and it gets in the way of how we decide to handle symbolic links. Hence, I think we should handle nested .gitignore using a different strategy.

I removed the feature from the changelog.

Test Plan

Added tests.

I did various tests by playing with the files.ignore, and making sure I got the correct diagnostics for the files Biome can't handle.

Copy link

netlify bot commented Dec 18, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 5f72026
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6580afe853b6050008e29cde

@ematipico ematipico requested review from a team December 18, 2023 17:16
@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Project Area: project A-Website Area: website A-Diagnostic Area: diagnostocis A-Changelog Area: changelog labels Dec 18, 2023
Cargo.toml Show resolved Hide resolved
biome.json Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it because the top .editorconfig already does the same

@ematipico ematipico merged commit 89f6002 into main Dec 18, 2023
20 checks passed
@ematipico ematipico deleted the chore/remove-ignore branch December 18, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-CLI Area: CLI A-Core Area: core A-Diagnostic Area: diagnostocis A-Project Area: project A-Website Area: website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Biome doesn't throw errors for files that can't handle
3 participants