workers-sdk
workers-sdk copied to clipboard
[Wrangler] Update the commands list
What this PR solves / how to test: This PR refreshes wrangler's default commands list, by removing old commands, updating visuals within the list, thus making it more consistent and easier to browse.
Before
After
Associated docs issue(s)/PR(s): https://github.com/cloudflare/cloudflare-docs/pull/11514
Author has included the following, where applicable:
- [x] Tests
- [ ] Changeset (Changeset guidelines)
Reviewer is to perform the following, as applicable:
- Checked for inclusion of relevant tests
- Checked for inclusion of a relevant changeset
- Checked for creation of associated docs updates
- Manually pulled down the changes and spot-tested
Note for PR author:
We want to celebrate and highlight awesome PR review! If you think this PR received a particularly high-caliber review, please assign it the label highlight pr review so future reviewers can take inspiration and learn from it.
🦋 Changeset detected
Latest commit: a61500f162a98a004aeab68e7bcb2fbc17c5dd8b
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 2 packages
| Name | Type |
|---|---|
| wrangler | Patch |
| @cloudflare/vitest-pool-workers | Patch |
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
A wrangler prerelease is available for testing. You can install this latest build in your project with:
npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-wrangler-3735
You can reference the automatically updated head of this PR with:
npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/3735/npm-package-wrangler-3735
Or you can use npx with this latest build directly:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-wrangler-3735 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-create-cloudflare-3735 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-cloudflare-kv-asset-handler-3735
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-miniflare-3735
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-cloudflare-pages-shared-3735
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9601015901/npm-package-cloudflare-vitest-pool-workers-3735
Note that these links will no longer work once the GitHub Actions artifact expires.
[email protected] includes the following runtime dependencies:
| Package | Constraint | Resolved |
|---|---|---|
miniflare |
workspace:* | 3.20240610.1 |
workerd |
1.20240610.1 | 1.20240610.1 |
workerd --version |
1.20240610.1 | 2024-06-10 |
Please ensure constraints are pinned, and miniflare/workerd minor versions match.
Codecov Report
Attention: Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.
Project coverage is 75.44%. Comparing base (
2789f26) to head (00c640f). Report is 296 commits behind head on main.
:exclamation: Current head 00c640f differs from pull request most recent head 8bd2bc7
Please upload reports for the commit 8bd2bc7 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #3735 +/- ##
==========================================
+ Coverage 72.44% 75.44% +2.99%
==========================================
Files 331 223 -108
Lines 17298 12332 -4966
Branches 4422 3188 -1234
==========================================
- Hits 12532 9304 -3228
+ Misses 4766 3028 -1738
| Files | Coverage Δ | |
|---|---|---|
| packages/wrangler/src/ai/index.ts | 100.00% <ø> (ø) |
|
| packages/wrangler/src/constellation/index.ts | 100.00% <100.00%> (ø) |
|
| packages/wrangler/src/hyperdrive/index.ts | 100.00% <ø> (ø) |
|
| packages/wrangler/src/index.ts | 83.72% <100.00%> (-6.59%) |
:arrow_down: |
| packages/wrangler/src/kv/index.ts | 100.00% <ø> (ø) |
|
| packages/wrangler/src/pages/index.ts | 84.00% <100.00%> (-2.21%) |
:arrow_down: |
| ...wrangler/src/queues/cli/commands/consumer/index.ts | 100.00% <ø> (+15.38%) |
:arrow_up: |
| packages/wrangler/src/queues/cli/commands/index.ts | 100.00% <100.00%> (ø) |
|
| packages/wrangler/src/secret/index.ts | 88.51% <ø> (+5.18%) |
:arrow_up: |
| packages/wrangler/src/vectorize/index.ts | 100.00% <ø> (ø) |
|
| ... and 1 more |
Great effort combing through all of this!
A couple subjective points:
- I think the emojis should have a space after them before the text begins
- I think using the same emoji for all commands reduces their "fun" quotient (I bet I sound fun right now 👀) and I don't think the consistency there is of any benefit
- I like the introduction of a consistent emoji for all experimental commands – maybe we could use a different one that's less subtle?
@RamIdeas thanks for the feedback! the emoji changes are based on a proposal from the design team -- i'll get them looped into the discussion. for my two cents, i agree that the consistency has a lower fun quotient 😄, but i do think it adds some informational value that the more assorted collection lacks: it signals that there is a loose relationship between the items in the different emoji groups, and also distracts less from the command descriptions.
cc @Meahaa
Excellent points let's:
- Add the space
- Keep the emoji's agree it's more fun to have them and we can take a pass in the future to make sure they actually make sense
- I'm not following too clearly here - I liked the orange text because it's a nice brand call out
- I'm not following too clearly here - I liked the orange text because it's a nice brand call out
I think you're referring to the "open beta" text in the figma design doc which I think being on-brand orange is great!
I was referring to the ⚑ flag emoji which blends into the text a little – a non-exhaustive set of alternatives to consider:
- 🧪 test tube
- 👩🔬 scientist
- 🥼 lab coat
- 🧫 petri dish
- 🔬 microscope
@Meahaa - can you take a look at @RamIdeas' last comment? Should we update that last emoji? @lrapoport-cf - can we rebase and fix the conflicts while updating to fit the agreed points between @RamIdeas and @Meahaa ?
ooh I like the test tube let's go with that!
Excellent points let's:
- Add the space
- Keep the emoji's agree it's more fun to have them and we can take a pass in the future to make sure they actually make sense
- I'm not following too clearly here - I liked the orange text because it's a nice brand call out
@Meahaa I am taking over this PR to cross it over the finish line. Can you please clarify point 2 for me? Do we keep the current emojis as they are, or do we change them as per design?
@Meahaa @RamIdeas @petebacondarwin I've added a before and after screenshot to the description of this PR. Before reviewing the code, is there consensus that, visually, things are as we want them to be? <3
I read that @Meahaa wrote:
Keep the emoji's agree it's more fun to have them and we can take a pass in the future to make sure they actually make sense
So I think that means we are all agreed that we should not switch out the mix of emojis for the boring few?
Yup! Let's just keep the current emojis for now! Agree with the fact that they make them more fun for now and we can always update to make them even better later!