-
Notifications
You must be signed in to change notification settings - Fork 275
feat: Add automatic git repository initialization to spin new
command
#3155
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: iamrajiv <[email protected]>
Signed-off-by: iamrajiv <[email protected]>
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 for this Rajiv! It basically looks good but I did have a few minor questions and nits. Hopefully all easy to address (and they may be my lack of understanding).
partial_file.context("Error scanning template partials directory")?; | ||
if !partial_file.file_type().is_ok_and(|t| t.is_file()) { | ||
anyhow::bail!("Non-file in partials directory: {partial_file:?}"); | ||
match self.template.partials_dir() { |
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.
It's not clear to me why partials support needs to change as part of this. Should this be a separate PR or is it needed here?
.await | ||
.understand_git_result() | ||
.map(|_| ()) | ||
.map_err(|e| anyhow::anyhow!("{}", e)) |
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 seems like it might be equivalent to git.output().await.understand_git_result()?;
- not sure but those maps seem a bit odd.
} | ||
|
||
if let Err(e) = self.init_git_repo(&target_dir).await { | ||
eprintln!("Warning: Failed to initialize a git repository for you: {}", e); |
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.
Use terminal::warn!
@@ -19,9 +19,6 @@ use crate::{ | |||
template::Template, | |||
}; | |||
|
|||
/// A set of partials to be included in a Liquid template. | |||
type PartialsBuilder = liquid::partials::EagerCompiler<liquid::partials::InMemorySource>; |
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.
Removing this is making the compile fail - I am not sure how you were even able to test it in the current state! Whatever the intent, I suggest we back out the partials changes from this PR - I cannot imagine any reason they'd need to change for git init support.
Fixes: #2580
FYI: