Skip to content
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

Replace cargo-generate with kickstart #38

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eupn
Copy link
Member

@eupn eupn commented Dec 22, 2020

Closes #36.

An example of prompts with defaults:

kickstart https://github.com/knurling-rs/app-template
Author's name? [default: You <[email protected]>]: 
How to you name your project's crate? [default: my-embedded-app, validation: ^([a-zA-Z][a-zA-Z0-9_-]+)$]: 
What's the name of the chip (see `probe-run --list-chips` output)? [default: nRF52840_xxAA]: 
What's the target's name?: 
  1. thumbv6m-none-eabi (Cortex-M0 and Cortex-M0+)
  2. thumbv7m-none-eabi (Cortex-M3)
  3. thumbv7em-none-eabi (Cortex-M4 and Cortex-M7 | no FPU)
  4. thumbv7em-none-eabihf (Cortex-M4F and Cortex-M7F | with FPU)
  > Choose from 1..4 [default: 1]: 
Do you want to use an embedded-hal?: 
  1. yes
  2. no
  > Choose from 1..2 [default: 1]: 
embedded-hal crate name? [default: nrf52840-hal]: 
embedded-hal crate version? [default: 0.11.0]: 

Everything done, ready to go!

Tested by compiling the generated project with defaults.

@Sympatron
Copy link

I think this would make it a bit easier to start.
Can you use a URL with kickstart for the template? With cargo generate you could specify --git https://github.com/knurling-rs/app-template so you don't have to clone the template first.

I think removing setup step 3 and 5 is fine, but the rest of the steps contain information that is still valuable. I would just change the order, because the user has to lookup chip name and HAL name and version before starting kickstart.

@eupn
Copy link
Member Author

eupn commented Dec 23, 2020

@Sympatron

Hello and thank you for the feedback!

Can you use a URL with kickstart for the template?

Yes, I've updated the README with a very similar command to use with kickstart:

$ kickstart https://github.com/knurling-rs/app-template

I would just change the order, because the user has to lookup chip name and HAL name and version before starting kickstart.

Yeah, it makes sense to first introduce the probe-run's capabilities in terms of supported chips before suggesting kickstart commands.

@japaric
Copy link
Member

japaric commented Jan 4, 2021

Thanks for the PR, @eupn. The UI looks good.

Something I found very surprising is that kickstart $URL copies the template contents into the current working directory instead of creating a new folder. Is there a way to make it generate a new folder like cargo-generate does? If not, let's please document that behavior in the README.

I noticed that kickstart shells out to git to fetch a git repository. We should document in the README that having git installed is a pre-requisite. Not all our users have git installed (e.g. on Windows) and we have already applied work around to make e.g. defmt work without the git tool so we should give users a "heads up" here.

I would prefer to keep the 'how to do this manually' info in the README as some people may prefer to download a zip version of this repository rather than clone it. We can divide the README in two sections: kickstart and "manual setup".

(Just thinking out loud: the other thing I noticed is that if you answer 'no' to 'do you want to use an embedded-hal?' then the generated project will fail to build with "memory.x file not found". I guess if one takes that path the template should also generate the memory.x file -- though answering how much Flash / RAM can require some digging and may be daunting for beginners. (Then again some HAL crates don't provide a memory.x either))

@eupn
Copy link
Member Author

eupn commented Jan 9, 2021

@japaric

Something I found very surprising is that kickstart $URL copies the template contents into the current working directory instead of creating a new folder. Is there a way to make it generate a new folder like cargo-generate does? If not, let's please document that behavior in the README.

You can specify the output directory via --output-dir <dir> flag. I'll include this in README for it to be less surprising.

I noticed that kickstart shells out to git to fetch a git repository. We should document in the README that having git installed is a pre-requisite. Not all our users have git installed (e.g. on Windows) and we have already applied work around to make e.g. defmt work without the git tool so we should give users a "heads up" here.

Sure, I'll document this as well.

I would prefer to keep the 'how to do this manually' info in the README as some people may prefer to download a zip version of this repository rather than clone it. We can divide the README in two sections: kickstart and "manual setup".

Yes, let's keep the manual steps and make two sections in README.

(Just thinking out loud: the other thing I noticed is that if you answer 'no' to 'do you want to use an embedded-hal?' then the generated project will fail to build with "memory.x file not found". I guess if one takes that path the template should also generate the memory.x file -- though answering how much Flash / RAM can require some digging and may be daunting for beginners. (Then again some HAL crates don't provide a memory.x either))

Not sure what would be the preferred approach here if the user doesn't want to use HAL crate. Probably we'll just let the user write the memory.x file themselves by consulting their HAL's docs.

README.md Outdated Show resolved Hide resolved
"thumbv6m-none-eabi (Cortex-M0 and Cortex-M0+)",
"thumbv7m-none-eabi (Cortex-M3)",
"thumbv7em-none-eabi (Cortex-M4 and Cortex-M7 | no FPU)",
"thumbv7em-none-eabihf (Cortex-M4F and Cortex-M7F | with FPU)"
Copy link
Contributor

Choose a reason for hiding this comment

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

We support thumbv8 too

Co-authored-by: Jonas Schievink <[email protected]>
@Urhengulas
Copy link
Member

Hi @eupn, Can you please rebase your PR and resolve the merge conflicts?

@eupn
Copy link
Member Author

eupn commented Jun 15, 2021

@Urhengulas sure!

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.

Replace cargo-generate with kickstart
5 participants