Skip to content

CC-5462: Simplify containers images list output#9620

Merged
CarmenPopoviciu merged 2 commits intocloudflare:mainfrom
gpanders:push-ttrkxyqwptkx
Jun 25, 2025
Merged

CC-5462: Simplify containers images list output#9620
CarmenPopoviciu merged 2 commits intocloudflare:mainfrom
gpanders:push-ttrkxyqwptkx

Conversation

@gpanders
Copy link
Copy Markdown
Member

@gpanders gpanders commented Jun 16, 2025

Fixes CC-5462.

Use a simple table (one row for each entry) instead of a fancy table with box characters (that is hard to read and breaks when text wraps). Print one row for each name:tag combination.


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: UI only change
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: UI only change
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: the cloudchamber images feature does not exist in v3

@gpanders gpanders requested review from a team as code owners June 16, 2025 18:59
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 16, 2025

🦋 Changeset detected

Latest commit: 2c7b227

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 16, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9620

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9620

miniflare

npm i https://pkg.pr.new/miniflare@9620

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9620

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9620

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9620

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9620

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9620

wrangler

npm i https://pkg.pr.new/wrangler@9620

commit: 2c7b227

@gpanders gpanders force-pushed the push-ttrkxyqwptkx branch 5 times, most recently from 66a8235 to 70d8c66 Compare June 16, 2025 22:29
Copy link
Copy Markdown
Contributor

@vicb vicb Jun 17, 2025

Choose a reason for hiding this comment

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

Is this equivalent to:

Suggested change
widths[i] = rows.map((r) => r[i].length).reduce((a, b) => Math.max(a, b), headers[i].length);
widths[i] = Math.max(headers[i].length, ...rows.map((r) => r[i].length));

Copy link
Copy Markdown
Member Author

@gpanders gpanders Jun 17, 2025

Choose a reason for hiding this comment

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

Almost, but using the spread operator can fail or return incorrect results if the array is too large (according to the MDN reference). reduce doesn’t have that problem.

It’s unlikely that this array will ever be so large that that’s an issue in practice, but given that the array is technically unbounded in size I went with the safer option.

Comment thread packages/wrangler/src/cloudchamber/images/list.ts Outdated
@gpanders gpanders force-pushed the push-ttrkxyqwptkx branch 2 times, most recently from d162379 to a7bfec5 Compare June 17, 2025 14:02
Use a simple table (one row for each entry) instead of a fancy table
with badly drawn box characters. Print one row for each name:tag
combination.
Copy link
Copy Markdown
Contributor

@CarmenPopoviciu CarmenPopoviciu left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jun 24, 2025
@CarmenPopoviciu CarmenPopoviciu added this pull request to the merge queue Jun 25, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 25, 2025
@CarmenPopoviciu CarmenPopoviciu added this pull request to the merge queue Jun 25, 2025
Merged via the queue into cloudflare:main with commit 1b967ea Jun 25, 2025
33 of 35 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jun 25, 2025
@gpanders gpanders deleted the push-ttrkxyqwptkx branch July 1, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants