search

cloudflare / wrangler-action

🧙‍♀️ easily deploy cloudflare workers applications using wrangler and github actions
Apache License 2.0
1.25k stars 159 forks source link

docs: remove warning about packageManager and pnpm default #245

Closed Cherry closed 7 months ago

Cherry commented 7 months ago

This effectively reverts https://github.com/cloudflare/wrangler-action/pull/242.

This is inferred by https://github.com/cloudflare/wrangler-action/pull/190 and then fixed in https://github.com/cloudflare/wrangler-action/pull/193, and I'm not really sure the "default" copy/pasted config should use pnpm - that's definitely not the standard.

The package manager you'd like to use to install and run wrangler. If not specified, the preferred package manager will be inferred based on the presence of a lockfile or fallback to using npm if no lockfile is found. Valid values are npm | pnpm | yarn | bun.

You do not need to specify packageManager as the docs now suggests, as long as you're running an up to date version.

If there is a bug in the package manager detection, I'd suggest that should be reported and addressed separately.

Cherry commented 7 months ago

cc @petebacondarwin

Cherry commented 7 months ago

Thanks for the feedback @IgorMinar. I believe everything should be addressed now.

Sadly the tests here will fail due to the necessary secrets not existing in the PR. Reference: https://github.com/cloudflare/wrangler-action/pull/216#issuecomment-1852411821

This PR only updates docs though, so should be safe to merge.

IgorMinar commented 7 months ago

Thanks James. This reads much better! I've merged the PR.