-
Notifications
You must be signed in to change notification settings - Fork 8
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
Initial work on performance #131
Conversation
Looks like there are just some unused-related warnings, plus Conventional Commits issues, and then it's ready to merge. Probably just squash down to one commit and reword the message to match Conventional Commits. |
…and replacing `filter_map` with `filter` and `map` in two places.
@alilleybrinker assuming this last check is successful (it should be) I think this is ready to review/merge! |
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.
I for one welcome our Arc
overlords
// If no subcommand matched, default to use of '-t <TYPE> <TARGET' syntax. In | ||
// this case, `target` is a required field, but the existence of a subcommand | ||
// removes that requirement |
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.
If this comment was intentionally duplicated, is there a good reason for doing so?
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.
This was unintentional! Probably happened while merging. Thanks for catching this!
use tempdir::TempDir; | ||
|
||
/// Handle unstable commands. | ||
pub fn main(args: UnstableArgs, config: &CliConfig) -> io::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.
I would stylistically opt for a different name, such as run
or run_unstable
, but not a big deal
This PR includes several commits, but the most major change is that
Rc
has been replaced withArc
throughout much of the repository, in order to prepare for improvements from multithreading.