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

upgrade to Yarn 4 and clean up some deps #99

Merged
merged 15 commits into from
Sep 7, 2024
Merged

upgrade to Yarn 4 and clean up some deps #99

merged 15 commits into from
Sep 7, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Sep 5, 2024

Lots of cleanup of dependencies. Principally:

  • Upgrading Yarn to v4
  • Adding a script to update all deps to match an NPM tag

@turadg turadg marked this pull request as ready for review September 6, 2024 19:29
@turadg turadg requested a review from toliaqat September 6, 2024 19:30
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I could use more docs on the script, but I'm willing to bet it's a lot better than the status quo, so I'm happy if they come after this PR.

@@ -0,0 +1,75 @@
#!/usr/bin/env node
Copy link
Member

Choose a reason for hiding this comment

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

Help me understand the workflow around this? How do I know when I should use it?

In general, if I want to yarn add foo, will I need this every time? or every time foo is @agoric/*? How about endo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add a note.

Copy link
Member

Choose a reason for hiding this comment

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

An entry in the scripts key of package.json would make it more discoverable, too. yarn fix-deps?

Copy link
Member Author

Choose a reason for hiding this comment

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

for now it's a "if you know you know" kind of tool. I think we need some design discussion around the UX to solve this problem consistently across repos. It should be something you can run with the agoric command or an npx so you don't need the script copied everywhere.

Comment on lines 13 to -15
"resolutions": {
"ses": "1.3.0",
"@endo/bundle-source": "2.5.2-upstream-rollup",
"@endo/captp": "3.1.1",
Copy link
Member

Choose a reason for hiding this comment

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

👏 excellent!

Get rid of resolutions-note too, please?

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe this looked like the resolutions are gone, but they're just updated. I hope to obviate them eventually but didn't manage yet

Copy link
Member

Choose a reason for hiding this comment

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

oops. reading too fast.

@@ -59,8 +59,10 @@ export const meta = {
{ maxItems: M.bigint() },
),
};
harden(meta);
Copy link
Member

Choose a reason for hiding this comment

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

is this enforced by this change? if so, cool!

Copy link
Member Author

Choose a reason for hiding this comment

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

by the updated Endo lint config

@@ -14,6 +14,6 @@
"author": "Agoric",
"license": "Apache-2.0",
"dependencies": {
"@agoric/cosmic-swingset": "^0.41.3"
"@agoric/cosmic-swingset": "^0.42.0-u16.2"
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't have to maintain _agstate like this. Marking this one as tech debt:

@turadg turadg merged commit 1f75e6a into main Sep 7, 2024
3 checks passed
@turadg turadg deleted the ta/packages branch September 7, 2024 00:23
dckc added a commit to dckc/dapp-ertp-airdrop that referenced this pull request Sep 10, 2024
node scripts/update-dependencies.js ses latest
node scripts/update-dependencies.js endo latest
node scripts/update-dependencies.js agoric agoric-upgrade-16

then copied some resolutions and devDependencies from dapp-offer-up,
esp. as of Agoric/dapp-offer-up#99

then

yarn && yarn dedupe
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.

2 participants