Skip to content

fix: semver comparison#216

Merged
petebacondarwin merged 3 commits intocloudflare:mainfrom
Cherry:fix/semver-check
Dec 13, 2023
Merged

fix: semver comparison#216
petebacondarwin merged 3 commits intocloudflare:mainfrom
Cherry:fix/semver-check

Conversation

@Cherry
Copy link
Contributor

@Cherry Cherry commented Dec 9, 2023

fixes #215
closes #214

This also bumps a few dependencies, to remove some npm audit warnings. The output bundle increases from 964kB to 983kB, but I feel the 19kB increase is worth the reliable semver comparison.

chore: bump dependencies
@Cherry Cherry requested a review from a team as a code owner December 9, 2023 18:29
@Cherry Cherry force-pushed the fix/semver-check branch 2 times, most recently from 2dbf0c7 to c559f04 Compare December 9, 2023 18:46
@Cherry
Copy link
Contributor Author

Cherry commented Dec 11, 2023

Not sure what's up with the test failure here, that's not one that I touched 🤔

- "Directory /does/not/exist does not exist."
+ [Error: Directory /does/not/exist does not exist.]

Seems like the test is doing what it expected, but not checking for the right error type?

@petebacondarwin
Copy link
Contributor

The failed test is due to updating vitest to v1, which contains this breaking change: vitest-dev/vitest#4396.

The resolution is just to fix up the snapshot to match the new expected value.

@Cherry
Copy link
Contributor Author

Cherry commented Dec 12, 2023

Great catch, thanks @petebacondarwin. I've updated that now and all should be good.

@petebacondarwin
Copy link
Contributor

Hmm. The failing tests need to be run on our fork since they require secrets...

@Cherry
Copy link
Contributor Author

Cherry commented Dec 12, 2023

Yeah I can't really fix that one unfortunately 😞 I've not made any real changes to the deploy process of this, but let me know if I can assist with anything further here to get this merged.

@petebacondarwin
Copy link
Contributor

Well that is interesting! By pushing a branch with the exact commit from this PR in it, the CI tests ran and Github attributed the successful test run to this PR too! TIL!

All good to go.

Thanks @Cherry

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

action doesn't use bulk secret upload when specifying wranglerVersion 3.10.0 or higher

3 participants