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.

Sign up for GitHub

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

Jump to bottom

fix(cloudflare): astro asset image service #33

Merged
merged 3 commits into from
Oct 24, 2023
Merged

fix(cloudflare): astro asset image service #33

merged 3 commits into from
Oct 24, 2023

Conversation

alexanderniebuhr
Copy link
Member

@alexanderniebuhr alexanderniebuhr commented Oct 20, 2023 •
edited
Loading

@changeset-bot
Copy link

changeset-bot bot commented Oct 20, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 8caacd1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@astrojs/cloudflare Patch
@test/astro-cloudflare-dev-runtime Patch
@test/astro-cloudflare-directory-mode Patch
@test/astro-cloudflare-function-per-route Patch
@test/astro-cloudflare-hybrid Patch
@test/astro-cloudflare-no-output Patch
@test/astro-cloudflare-prerender Patch
@test/astro-cloudflare-routes-json Patch
@test/astro-cloudflare-wasm-function-per-route Patch
@test/astro-cloudflare-wasm-directory Patch
@test/astro-cloudflare-wasm Patch
@test/astro-cloudflare-with-solid-js Patch
@test/astro-cloudflare-wrangler-runtime 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

@alexanderniebuhr alexanderniebuhr mentioned this pull request Oct 21, 2023
Merged
dreyfus92
dreyfus92 requested changes Oct 21, 2023
View reviewed changes
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

I hope this helps @alexanderniebuhr

.changeset/nice-impalas-whisper.md Outdated Show resolved Hide resolved
@sarah11918
Copy link
Member

I like everyone's suggestions! 😄 Some include more detail than others, and I honestly think either way works. (I do have a preference for starting with a verb like "Fixes" though, as I think that sets the reader up for what kind of change this is, and how important it might be to their own code/project.)

@alexanderniebuhr pick what you like now that you've seen some suggestions that describe the problem that existed (that this PR addresses) and ping me again for a final proof!

@alexanderniebuhr @sarah11918
@VoxelMC @dreyfus92
resolve review comments
8caacd1 
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Voxel <20650404+VoxelMC@users.noreply.github.com>
Co-authored-by: Paul Valladares <85648028+dreyfus92@users.noreply.github.com>
dreyfus92
dreyfus92 approved these changes Oct 22, 2023
View reviewed changes
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

LGTM ✅

sarah11918
sarah11918 approved these changes Oct 22, 2023
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

LGTM!

ematipico
ematipico approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

It would be great if you could tell us how you tested the fix

@ematipico ematipico merged commit 78baf24 into main Oct 24, 2023
8 checks passed
@ematipico ematipico deleted the nbhr- branch October 24, 2023 09:30
@github-actions github-actions bot mentioned this pull request Oct 24, 2023
Merged
@alexanderniebuhr
Copy link
Member Author

@ematipico Oops, my bad! I tested the changes locally using a demo project and sprinkled in some console.log statements for clarity. I'll definitely include those test descriptions in upcoming PRs. Thanks for the heads-up! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VoxelMC VoxelMC VoxelMC left review comments

@ematipico ematipico ematipico approved these changes

@sarah11918 sarah11918 sarah11918 approved these changes

@dreyfus92 dreyfus92 dreyfus92 approved these changes

Assignees
No one assigned
Labels
pkg: cloudflare
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alexanderniebuhr @sarah11918 @ematipico @VoxelMC @dreyfus92