-
-
Notifications
You must be signed in to change notification settings - Fork 735
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
Add action to move imports to top of module #3413
base: main
Are you sure you want to change the base?
Conversation
e6a47c2
to
8843508
Compare
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.
Thanks! I've left a few notes inline as I think we need to cover things like import grouping and comments.
Ideally the splitting would have been in its own PR. Having two things in one PR means one will get held back by the other and it's hard to review.
|
||
pub fn main() {} | ||
|
||
import result |
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.
What happens if the imports are lower but in groups? Do the groups get preserved?
What about comments?
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.
Mmh this is quite tricky, basically I'd have to reimplement part of the pretty printer here but I'm not a fan of that... I'll have to think a bit more how to approach this
2b6b8ee
to
b42ff26
Compare
This PR closes #3186
I also moved this action and the tuple one to their own module because it was starting to feel a bit overwhelming to have everything in the actions module. If you don't like this I can put everything back to just that file though!