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

Minor (potential) improvements #16

Closed
wants to merge 1 commit into from

Conversation

Rolv-Apneseth
Copy link

Feel free to decide if this is even necessary but these are the changes in this small PR:

  • Format using rustfmt
  • Fix clippy warnings
  • Put all tests in separate modules

I suppose the main thing is the tests going into modules, as that cleans up the test specific imports and puts all tests behind a #[cfg(test)] attribute which should keep all those test functions from needing to be included in the final compilation even when not testing. This seemed to lead to saving an admittedly tiny 2k (48k to 46k) from the libshlex.rlib with cargo build --lib --release on my machine.

@fenhl
Copy link
Collaborator

fenhl commented Nov 14, 2023

Please split these changes into a separate PR for each bullet point so they can be reviewed individually.

@Rolv-Apneseth
Copy link
Author

Sure yeah. Would it be alright to keep the formatting and linting stuff together, so it's 2 separate PRs?

@fenhl
Copy link
Collaborator

fenhl commented Nov 14, 2023

I'd prefer 3 PRs since it's the formatting stuff that I'm least inclined to accept currently.

@Rolv-Apneseth
Copy link
Author

Alright no problem. I'll close this for now and get to it hopefully later tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants