Skip to content

Conversation

@itowlson
Copy link
Collaborator

Fixes #2580

This picks up on work done in #3155 by @iamrajiv. The core of that PR was fine, but unfortunately there were some unrelated blocking problems, and the PR has since gone inactive.

.stdout(Stdio::null())
.stderr(Stdio::null());

let status = cmd.status().await?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use of context would likely help here as the error message from the status call is likely to not be very helpful out of context.

}
}

async fn initialise_git(&self) -> anyhow::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
async fn initialise_git(&self) -> anyhow::Result<()> {
async fn maybe_initialise_git(&self) -> anyhow::Result<()> {


let target_dir = self.generation_target_dir();

let skip_initing_repo = git::is_in_git_repo(&target_dir).await.unwrap_or(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we warn if is_in_git_repo errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say not: we don't want to error on "git not installed," and other cases seem unlikely. We could trace or something if you feel strongly?

Copy link

@iamrajiv iamrajiv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm sorry i missed the notice for the review comment @itowlson thanks for the fix

@adamreese
Copy link
Contributor

@itowlson Rebasing on main will fix the CI errors now

@itowlson itowlson force-pushed the git-init-in-new-app branch from 9e409e0 to 44ab8b2 Compare July 14, 2025 19:58
@itowlson itowlson enabled auto-merge July 14, 2025 20:01
@itowlson itowlson merged commit e296edc into spinframework:main Jul 14, 2025
17 checks passed
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.

Add version control support to spin new or spin CLI

4 participants