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

Update get-started.mdx #16322

Merged
merged 6 commits into from
Sep 6, 2024
Merged

Update get-started.mdx #16322

merged 6 commits into from
Sep 6, 2024

Conversation

swarajbachu
Copy link
Contributor

the previous command wasnt actually creating it to the framework as next, changing the tag is initializing the next project

Summary

the command was about starting a new next project, but it is instead just running to create a normal project

Screenshots (optional)

previous

Screenshot 2024-08-20 at 8 35 44 PM

the previous command wasnt actually creating it to framework as nextjs, changing the tag is initializing next repo
@swarajbachu
Copy link
Contributor Author

hello @IgorMinar can you look into this once

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Sep 4, 2024

Thanks for the PR @swarajbachu 🙂

Could you please update your PR to use the PacakgeManager starlight component (see starlight-package-managers) we're now using across the docs?

Something like:

<PackageManagers type="create" pkg="cloudflare@latest" args="--framework=next" />

That will make sure that we do get the right command for the various package managers 🙂

PS: if you could apply the change also to all the other guides under src/content/docs/pages/framework-guides that would be really really appreciated! 😁 🫶

@swarajbachu
Copy link
Contributor Author

sure, lemme do that

@swarajbachu
Copy link
Contributor Author

swarajbachu commented Sep 5, 2024

starlight-package-managers

just one thing to note is that this means it will appear
this way

npm create cloudflare@latest -- --framework=next

instead of

npm create cloudflare@latest my-next-app -- --framework=next

or I can do

<PackageManagers type="create" pkg="cloudflare@latest my-next-app" args="--framework=next" />

but it doesn't seem to be a good practice

so for now I am doing the way you told me to do, lemme know if you need to change it further

@github-actions github-actions bot added size/m and removed size/xs labels Sep 5, 2024
@swarajbachu
Copy link
Contributor Author

@dario-piotrowicz updated the way you mentioned for now

@dario-piotrowicz
Copy link
Member

@swarajbachu thanks so much for updating the guides, it's really very much appreciated!

As you pointed out in your comment my suggestion wasn't actually fully correct (I did it on the spot as an example without checking things too much, sorry for the confusion)

I think that the right change would then be:

<PackageManagers
	type="create"
	pkg="cloudflare@latest"
	args="my-next-app --framework=next"
/>

This does seem to produce the correct results:
Screenshot 2024-09-05 at 12 30 38
Screenshot 2024-09-05 at 12 30 45

and it's not, as far as I can tell, bad practice or anything like that.

What do you think? could you make this one last change? (to all the guides as well 😅) 🙏

@swarajbachu
Copy link
Contributor Author

swarajbachu commented Sep 5, 2024

@swarajbachu thanks so much for updating the guides, it's really very much appreciated!

As you pointed out in your comment my suggestion wasn't actually fully correct (I did it on the spot as an example without checking things too much, sorry for the confusion)

I think that the right change would then be:

<PackageManagers
	type="create"
	pkg="cloudflare@latest"
	args="my-next-app --framework=next"
/>

This does seem to produce the correct results: Screenshot 2024-09-05 at 12 30 38 Screenshot 2024-09-05 at 12 30 45

and it's not, as far as I can tell, bad practice or anything like that.

What do you think? could you make this one last change? (to all the guides as well 😅) 🙏

hmm yeah cool, now I am will do that, and do you want me to also add bun?

@dario-piotrowicz
Copy link
Member

hmm yeah cool, now I am will do that

@swarajbachu thanks a lot! ❤️

and do you want me to also add bun?

I'm not sure 😕 (I guess it depends how much extra code that would entail)

I'd avoid that for now, the best person to ask would be @KianNH, but he's currently on holiday I believe

I'm not sure if Kian has already considered/looked into bun, to avoid delays with merging this PR I would just avoid adding bun and potentially think of such addition as a followup 🙂

@swarajbachu
Copy link
Contributor Author

hmm yeah cool, now I am will do that

@swarajbachu thanks a lot! ❤️

and do you want me to also add bun?

I'm not sure 😕 (I guess it depends how much extra code that would entail)

I'd avoid that for now, the best person to ask would be @KianNH, but he's currently on holiday I believe

I'm not sure if Kian has already considered/looked into bun, to avoid delays with merging this PR I would just avoid adding bun and potentially think of such addition as a followup 🙂

cool lemme do the remaining changes then

@swarajbachu
Copy link
Contributor Author

@dario-piotrowicz done dude

@swarajbachu
Copy link
Contributor Author

hello dude @dario-piotrowicz

@dario-piotrowicz
Copy link
Member

@swarajbachu sorry I was unable to review the PR yesterday 🙇, I'm having a look right now 🙂

Copy link
Member

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Fantastic job @swarajbachu, thanks so very much for the various updates! 🫶

I've left a few comments for things that need amending, besides these small changes it looks great to me 😄

Co-authored-by: Dario Piotrowicz <[email protected]>
@kodster28
Copy link
Contributor

kodster28 commented Sep 6, 2024

Fantastic job @swarajbachu, thanks so very much for the various updates! 🫶

I've left a few comments for things that need amending, besides these small changes it looks great to me 😄

Thanks for the contribution, @swarajbachu, I'll merge as soon as checks pass. Definitely appreciate the help here making sure other folks don't run into the issue you experienced.

@kodster28 kodster28 merged commit 60da6f9 into cloudflare:production Sep 6, 2024
6 checks passed
@workers-devprod workers-devprod added the contribution [Holopin] Recognizes a docs contribution, big or small label Sep 6, 2024
Copy link

holopin-bot bot commented Sep 6, 2024

Congratulations @swarajbachu, the maintainer of this repository has issued you a holobyte! Here it is: https://holopin.io/holobyte/cm0qzwju811720cjqvvwfw1h5

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution [Holopin] Recognizes a docs contribution, big or small product:pages size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants