-
Notifications
You must be signed in to change notification settings - Fork 640
spacetime init rewrite
#3366
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: master
Are you sure you want to change the base?
spacetime init rewrite
#3366
Conversation
c022d34 to
ea0f0f7
Compare
ea0f0f7 to
e536cc3
Compare
10f14ee to
cc118cc
Compare
It was added first when the requirements were different. At the moment we only allow to chose server-lang if template is not provided
dfa4da7 to
db22a7f
Compare
…us/spacetime-init
Signed-off-by: Tyler Cloutier <[email protected]>
| } | ||
|
|
||
| let cursorrules_content = embedded::get_cursorrules(); | ||
| let cursorrules_path = project_path.join(".cursorrules"); |
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.
the cursorrules_content should go in .cursor/rules/spacetime-rules.mdc. (kebab case is preferred)
|
|
||
| ```bash | ||
| spacetime init --lang csharp my-project-directory | ||
| spacetime init --non-interactive --name my-spacetimedb-project --server-lang csharp my-project-directory |
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.
Several suggestions:
--name my-spacetimedb-project my-spacetimedb-directoryis a little redundant, let's make it so that the parameter is called--project-pathinstead and that parameter tells you where you're going put the project. Let's make the positional argument the project name (more in line with whatspacetime publishdoes). Let's make it so that if you do not specify a project path, it creates a directory in the current directory with the same name as the project name.- Let's leave off
--non-interactivein the docs, so it might actually go into interactive mode for some stuff. - Let's rename
--server-langto--lang(I'm less sure about this change. I would be okay to keep it--server-langif we made the other changes).
So this line would become:
spacetime init --lang csharp my-spacetimedb-projectThere 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.
Some UX questions/nits about github templates, I'm still reviewing
| template_type: TemplateType::GitHub, | ||
| server_lang: None, | ||
| client_lang: None, | ||
| github_repo: Some(template_str.to_string()), |
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.
Should we check to see if this looks like a URL here? Like if someone just mistypes a template name we will end up here too no?
| format!("https://github.com/{}", repo_input) | ||
| } else { | ||
| anyhow::bail!("Invalid repository format. Use 'owner/repo' or full URL"); | ||
| }; |
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 someone wants to use a template from a private repo or they just want to use SSH auth I think it would be nice to support that here. e.g. if someone passed this as their template URL:
# Either of these should work
[email protected]/clockworklabs/PrivateTemplate
ssh://[email protected]/clockworklabs/PrivateTemplate
| } | ||
|
|
||
| fn clone_github_template(repo_input: &str, target: &Path) -> anyhow::Result<()> { | ||
| let repo_url = if repo_input.starts_with("http") { |
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 wouldn't catch URLs like:
github.com/clockworklabs/MyTemplate
Also do we want to support pointing to local repo as well? Something like:
../clockwork/SomeTemplateImTesting
| } | ||
| } | ||
|
|
||
| fn slugify(name: &str) -> String { |
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.
Do we not just want to use the convert_case crate for this instead? Then you can just:
name.to_case(Case::Kebab)
Plus we already use this dependency anyway
| 1 => Some("pnpm"), | ||
| 2 => Some("yarn"), | ||
| 3 => Some("bun"), | ||
| _ => None, |
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 think if the user selected "other" here we should prompt them for the package manager they want to use
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 quite so simple, I think in this case they are doing something that 99.9% of users won't do, so we should leave them to their own devices.
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.
Let's remove the "other" option then?
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.
More review
| println!(" {} - {}", template.id, template.description); | ||
| } | ||
| println!(); | ||
| loop { |
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 loop isn't used
This is a draft of the new functionality for
spacetime init. In order to run it with built-in templates you have to set the path to the config file:In the future it will fetch the list from GH.
A few notes:
spacetime initdoes not work at the momentspacetimedbdirectory for the server filesmissing --foo, we will automatically launch interactive mode, which is harder to debug. That's why I think I'd prefer to implement--non-interactiveargument--langor--project-pathexplicitly, I guess we could run the legacy workflow, but not sure if it's worth it, as the command was marked as unstable anyway