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.

Sign up for GitHub

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

Jump to bottom

refactor(linter): refactor LintBuilder to prep for nested configs #8034

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

refactor(linter): refactor LintBuilder to prep for nested configs #8034

wants to merge 1 commit into from
+103 −117

Conversation

camc314
Copy link
Contributor

@camc314 camc314 commented Dec 19, 2024 •
edited
Loading

More simplification/preparations for nested configurations:

  1. renames LinterBuilder to ConfigStoreBuilder
  2. moves the options logic out of LintBuilder
  3. make ConfigStoreBuilder::build() return a result (currently always returns OK, but it will return errors when nested config is implemented

The next steps to implement nested config which i hope to do in the next week are:

  1. refactor the from_oxlintrc to accept a file path
  2. introduce a new method on ConfigStoreBuilder (name TBC) that walks all child directories, collecting .oxlintrc files. these will be put into ConfigStore as a hash map of path > config.

This was referenced Dec 19, 2024
Merged
Merged
@camc314 Graphite App
Copy link
Contributor Author

camc314 commented Dec 19, 2024 •
edited
Loading

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Dec 19, 2024
@camc314 camc314 marked this pull request as ready for review December 19, 2024 22:45
@camc314 camc314 force-pushed the c/12-19-refactor_linter_renamce_flat.rs_to_config.rs_ branch from 73d39ed to f6523f6 Compare December 19, 2024 22:54
@camc314 camc314 force-pushed the c/12-19-refactor_linter_refactor_lintbuilder_to_prep_for_nested_configs branch from ae07fda to 42adf86 Compare December 19, 2024 22:54
@Boshen Boshen requested a review from camchenry December 20, 2024 02:38
@camchenry
Copy link
Contributor

Thanks for taking a look at this, I'll review this as soon as possible. I had a draft of the nested config work on my laptop but wasn't happy with it so far.

@camc314
Copy link
Contributor Author

camc314 commented Dec 20, 2024

Thanks for taking a look at this, I'll review this as soon as possible. I had a draft of the nested config work on my laptop but wasn't happy with it so far.

ah sorry! i didn't realize you had already started implementing this.
feel free to close this one in favour of yours!

camchenry
camchenry approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@camchenry camchenry left a comment

Choose a reason for hiding this comment

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

nice!

crates/oxc_linter/src/config/config_builder.rs Outdated Show resolved Hide resolved
@camc314 camc314 force-pushed the c/12-19-refactor_linter_renamce_flat.rs_to_config.rs_ branch from f6523f6 to 0487198 Compare December 20, 2024 19:53
@camc314 camc314 force-pushed the c/12-19-refactor_linter_refactor_lintbuilder_to_prep_for_nested_configs branch 3 times, most recently from 30c712c to 0eeb0e0 Compare December 20, 2024 19:56
@Boshen
Copy link
Member

Boshen commented Dec 21, 2024

You may merge after fixing the compile error.

@Boshen Boshen changed the base branch from c/12-19-refactor_linter_renamce_flat.rs_to_config.rs_ to graphite-base/8034 December 21, 2024 02:02
@Boshen Boshen force-pushed the c/12-19-refactor_linter_refactor_lintbuilder_to_prep_for_nested_configs branch from 0eeb0e0 to e46b6e7 Compare December 21, 2024 02:08
@Boshen Boshen force-pushed the graphite-base/8034 branch from 0487198 to 952d7e4 Compare December 21, 2024 02:08
@Boshen Boshen changed the base branch from graphite-base/8034 to main December 21, 2024 02:09
@Boshen Boshen force-pushed the c/12-19-refactor_linter_refactor_lintbuilder_to_prep_for_nested_configs branch from e46b6e7 to 263abe6 Compare December 21, 2024 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camchenry camchenry camchenry approved these changes

Assignees

@camc314 camc314

Labels
A-cli Area - CLI A-editor Area - Editor and Language Server A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@camc314 @camchenry @Boshen