-
Notifications
You must be signed in to change notification settings - Fork 93
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 some git utilities #2095
Add some git utilities #2095
Conversation
Bencher Report
Click to view all benchmark results
|
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.
Unrelated, but as the number of our crates grow, I wonder if we should put them under a common crate
directory, because it's not very clear anymore what is a crate and what isn't, I guess
git/src/lib.rs
Outdated
.wrap_err()? | ||
// For now, we always fetch shallow. Maybe for the index it's more efficient to | ||
// keep a single repo around and update it? But that might be in another method. | ||
.with_shallow(gix::remote::fetch::Shallow::DepthAtRemote( |
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.
Nitpick: there are a lot of gix::remote::fetch::
prefixes. Maybe it would be worth importing at least fetch
, or even a few symbols directly?
std::fs::create_dir_all(dir).with_path(dir)?; | ||
|
||
// Fetch the git directory somewhere temporary. | ||
let git_tempdir = tempfile::tempdir().wrap_err()?; |
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.
Maybe a naive question, but why do we need a temporary directory first? Can't we do everything in dir
directly?
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.
We need to put the .git
stuff somewhere. There's probably no harm in putting it in dir
, but I thought it was cleaner to just populate dir
with the contents, and not the .git
stuff.
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.
Oh, you can choose to put the git stuff separately, I see. That's fine by me 👍
This adds a mini-crate with some git-related utilities. It's split off from the main packaging PR, and doesn't interact with anything else.