feat: useExportsLast #4172
feat: useExportsLast #4172
Conversation
github-actions
bot
added
A-Project
crates/biome_js_analyze/tests/specs/nursery/useExportsLast/valid_multiple_separate_exports.js
Outdated
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #4172 will not alter performanceComparing Summary
|
|
@togami2864 Thank you so much for the feedback. I'm pretty new to rust and biome and I appreciate the guidance. I think I've addressed all your concerns and I also resolved the merge conflicts including re-running |
tommymorgan
force-pushed
the
feat/useExportsLast
branch
2 times, most recently
from
e832245
to
cd1b1c6
Compare
tommymorgan
force-pushed
the
feat/useExportsLast
branch
from
cd1b1c6
to
97178a1
Compare
There was a problem hiding this comment.
We're almost there. One thing to address regarding the diagnostics. cc @togami2864 for visibility
| markup! { | ||
| "Export statements should appear at the end of the file." | ||
| }, | ||
| ) | ||
| .note(markup! { | ||
| "Move this export to the end of the file." | ||
| }), |
There was a problem hiding this comment.
In our contribution guide, we explain how to explain a diagnostic to the user. In this case, we don't explain the error. Can you address that, please?
There was a problem hiding this comment.
I made an update that I hope is acceptable. I had previously read the contribution guide but I just missed the mark. Thank you for providing such detailed docs though. That and the PR help has really made making my first constribution approachable.
There was a problem hiding this comment.
I believe there's been a misunderstanding. The previous message was fine, even better than the new one. What was missing is explain why exports should be moved, because that's the error. So the diagnostic should provide three notes:
- Error
- Explain the error
- Provide an actionable solution
Co-authored-by: Emanuele Stoppa <[email protected]>
Summary
This implements similar functionality to eslint-plugin-import/exports-last
Test Plan
Added tests and snapshots