这是indexloc提供的服务,不要输入任何密码
Skip to content

feat: init new git repo for new projects #558

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

Merged
merged 26 commits into from
Apr 8, 2025

Conversation

jacobhq
Copy link
Contributor

@jacobhq jacobhq commented Feb 12, 2025

Checklist

Related issue

Fixes #554

Overview

Checks if git is installed, and init a new repo by default. If git is not installed, we continue in silence, and only fail with error if the user specifies the git flag, but git is not installed.

Tasks

  • Ensure git is available in CI to run tests
  • Write unit test for function
  • Write integration test for all cases
  • Add flag to docs (not much to do, but will update the help snippet on the cli page)

@github-actions github-actions bot added the rust Requires rust knowledge label Feb 12, 2025
@jacobhq

This comment was marked as resolved.

@marcalexiei

This comment was marked as outdated.

@marcalexiei

This comment was marked as resolved.

@jacobhq

This comment was marked as resolved.

@jacobhq jacobhq marked this pull request as ready for review February 12, 2025 19:43
Copy link
Collaborator

@marcalexiei marcalexiei left a comment

Choose a reason for hiding this comment

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

I added a few comments to refine messages and comments.
Let me know what do you think 😉

(The code review will still be performed by @Valerioageno since I'm not a rust expert 😅)

@jacobhq
Copy link
Contributor Author

jacobhq commented Feb 12, 2025

I added a few comments to refine messages and comments.

Thank you! What do you think of my proposed logic for the git workflow (description in PR overview)?

@jacobhq
Copy link
Contributor Author

jacobhq commented Feb 13, 2025

Please can I have a bit of help here? When I use TempTuonoProject in the cli_build.rs file, there are no errors. However, because the rust compiler treats these tests separately, when I use the module in the cli_new.rs file, the compiler errors because in this test, the function is not used.

I could ignore the dead code warning, but that feels bad. WDYT?

@jacobhq
Copy link
Contributor Author

jacobhq commented Feb 14, 2025

@Valerioageno should I just remove some of these tests or comment them out/disable them so they can be rewritten once we have a mock?

@Valerioageno
Copy link
Member

Right now the tests are not reliable enough to be merged since they strongly rely on the host machine that run them (in a machine that doesn't have installed git they just fail).

I think that for this iteration we could:

  1. merge this PR without tests
  2. Complete the other PR (let me know if you are interested in taking it over)
  3. Add tests for this scenario

WDYT?

@jacobhq
Copy link
Contributor Author

jacobhq commented Feb 15, 2025

Should we put this on hold until we get the mock (I'm happy to help with that if needed), and then merge those changes into this branch and refactor the tests as needed?

I think it's a bit risky to merge this one without tests.

@Valerioageno
Copy link
Member

Makes sense! The goal with the other PR is to find a library that supports configuration at build time. When we have that we can proceed with this then! :)

@Valerioageno
Copy link
Member

This PR is unblocked now!

@jacobhq jacobhq marked this pull request as draft February 25, 2025 21:18
@jacobhq
Copy link
Contributor Author

jacobhq commented Feb 25, 2025

So to be clear, the only thing left to do is write some kind of mock for git?

@Valerioageno
Copy link
Member

Exactly, otherwise the test is dependant on the software installed on the running machine.

@Valerioageno
Copy link
Member

Let me know @jacobhq if you need support with this! :)

@jacobhq jacobhq marked this pull request as ready for review April 7, 2025 15:12
Copy link
Member

@Valerioageno Valerioageno left a comment

Choose a reason for hiding this comment

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

I'd easier down it even more.

Let's remove the --git-init flag (all the installations also init git if existing).
This way we can also skip the check is_git_installed and simply run the command at the end of the routine.

Let's then remove the error handling in init_new_git_repo and simply check trace every error with a meaningful message.

WDYT?

@jacobhq
Copy link
Contributor Author

jacobhq commented Apr 7, 2025

Oh ok, so just run the command and use the trace macro if it fails?

@Valerioageno
Copy link
Member

Yeah, I'd keep it super minimal

@jacobhq jacobhq requested a review from Valerioageno April 7, 2025 20:17
@jacobhq jacobhq requested a review from Valerioageno April 8, 2025 15:32
@Valerioageno Valerioageno merged commit b175210 into tuono-labs:main Apr 8, 2025
29 checks passed
@jacobhq jacobhq deleted the jm-cli-git-init branch April 8, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Requires rust knowledge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic git init
3 participants