-
-
Notifications
You must be signed in to change notification settings - Fork 63
feat(cli): Create & add one go #695
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
🦋 Changeset detectedLatest commit: e66e861 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Thank you for the initial work.
Let's speak over the different comments.
|
Hello @kizivat do you want to puch forward this PR? or should I try to bring it to the finish line ? :) |
commit: |
|
I'm not sure why |
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.
Not too sure about the comments, feel free to correct me if i missed anything.
Also the --install pnpm flag does not seem to be working now, probably because it detects an empty directory or something. Don't manage to understand why it stopped working though
| // add verifications and inter-addon deps | ||
|
|
||
| // add inter-addon dependencies |
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.
duplicated comment
|
|
||
| const setups = selectedAddons.map(({ addon }) => addon); | ||
| const setupResult = setupAddons(setups, workspace)[addon.id]; | ||
| workspace = virtualWorkspace || (await createWorkspace({ ...workspace })); |
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.
not sure this is the right call here. Previously we createWorkspace after every run, so that if tailwind and vitest run (after each other), the vitest run should already know via the workspace that tailwind is already installed.
Or am I missing something here?
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.
Actually not sure why that works, but that seems to work
| addonSetupResults?: Record<string, AddonSetupResult>; | ||
| virtualWorkspace?: Workspace<any>; | ||
| }): Promise<{ nextSteps: string[]; packageManager?: AgentName | null }> { | ||
| let workspace = virtualWorkspace || (await createWorkspace({ cwd: options.cwd })); |
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.
Same as above? in which case would virtualWorkspace even be undefined? Couldnt we just do this check once and make sure it's not undefined later on.
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.
You are 100% right!
I will refactor this later 👍
This should resolve #689 .
--addvariadic option tocreateenabling specifying add-ons during project creationPoint 1. is solved by separating the question prompting and applying add-ons in the
addscript. Both are reused in thecreateaction handler.I'm creating this as a draft PR to receive general direction before solving
// TODO_ONE:s.