-
-
Notifications
You must be signed in to change notification settings - Fork 721
feat(linter): introduce ExternalLinter struct
#12052
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(linter): introduce ExternalLinter struct
#12052
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. |
CodSpeed Instrumentation Performance ReportMerging #12052 will improve performances by 15.02%Comparing Summary
Benchmarks breakdown
|
d702a5b to
a3f0e94
Compare
33f6645 to
f7d998b
Compare
a3f0e94 to
bad4448
Compare
bad4448 to
da27091
Compare
da27091 to
b09422c
Compare
b09422c to
a295054
Compare
overlookmotel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the linker error on Linux, all looks good.
My only comment is that maybe it'd be preferable to keep napi out of oxc_linter crate.
I'm not really clear on where the split of responsibilities is between apps/oxlint and crates/oxc_linter, or why we have that separation. But since we do have that separation, it might be preferable to keep napi stuff in the napi/oxlint2 package.
Could ExternalLinter contain normal tokio async functions, and napi/oxlint2 creates an ExternalLinter containing async functions which call the ThreadsafeFunctions internally?
Note: I don't propose changing this PR, but just a thought for next iteration.
Merge activity
|
a295054 to
f81d336
Compare

No description provided.