Skip to content

Conversation

@yusinto
Copy link
Contributor

@yusinto yusinto commented Jul 17, 2023

Updating semver to latest. The reason it was pinned to 7.5.0 was because 7.5.0 introduced require('util') without the node: prefix. This has been fixed in 7.5.1 so we'll update to latest which is 7.5.4.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #209876: chore: update semver dep.

@kinyoklion
Copy link
Member

What confidence level do we have that this will execute nicely in cloudflare, vercel, akamai? Are the current unit tests sufficiently representative of those environments?

@dariushar @ctawiah @monsagri

Copy link
Member

@kinyoklion kinyoklion left a comment

Choose a reason for hiding this comment

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

Requesting changes until my question is addressed.

@louis-launchdarkly louis-launchdarkly dismissed their stale review July 17, 2023 18:26

Ryan has an open question on this.

@yusinto yusinto requested review from dariushar and monsagri July 17, 2023 18:58
Copy link
Contributor

@dariushar dariushar left a comment

Choose a reason for hiding this comment

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

Someone else may be a little more confident than I am but I took a quick look at the Akamai and Vervel SDKs and I don't see any issues being caused by this

@ctawiah
Copy link
Contributor

ctawiah commented Jul 18, 2023

What confidence level do we have that this will execute nicely in cloudflare, vercel, akamai? Are the current unit tests sufficiently representative of those environments?

@dariushar @ctawiah @monsagri

LGTM

@monsagri
Copy link
Contributor

Looks good to me from what I can see!

@yusinto yusinto requested a review from kinyoklion July 19, 2023 18:12
@yusinto yusinto merged commit 8087dd9 into main Jul 19, 2023
@yusinto yusinto deleted the yus/sc-209876/chore-update-semver-dep branch July 19, 2023 18:35
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.

7 participants