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

Publishing site assets include/exclude support #270

Merged
merged 7 commits into from
Jan 24, 2022

Conversation

petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Jan 19, 2022

This PR adds support for include/exclude options when publishing site assets.
It also fixes the output of kv:key list.
There are also a few refactorings to prepare the way for this.

QUESTION: Do we want to add the site-include and site-exclude CLI args to publish? I like the consistency that creates with our move toward CLI oriented development (not relying upon config) but since Sites assets are somewhat legacy, we may not want to invest in that public API? cc @threepointone

@changeset-bot
Copy link

changeset-bot bot commented Jan 19, 2022

🦋 Changeset detected

Latest commit: 26b91d4

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

This PR includes changesets to release 1 package
Name Type
wrangler 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

The `fetchInternal()` function assumes that the request is a GET if none is specified. This change brings the `mockFetchInternal()` function in line with this.
The previous output was passing an array of objects to console.log, which ended up showing something like

```
[Object object]
[Object object]
...
```

Now the result is JSON stringified before being sent to the console.
The tests have been fixed to check this too.
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

oh ha, I reviewed this in #277

packages/wrangler/src/config.ts Outdated Show resolved Hide resolved
Previously we were checking whether the base64 encoded size of an asset was too large (>25MiB).
But base64 takes up more space than a normal file, so this was too aggressive.
@petebacondarwin petebacondarwin merged commit 522d1a6 into cloudflare:main Jan 24, 2022
@petebacondarwin petebacondarwin deleted the sites-validation branch January 24, 2022 14:48
@github-actions github-actions bot mentioned this pull request Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants