Skip to content

Use more Yarn workspaces in scripts #7391

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
Merged
merged 13 commits into from
Apr 16, 2025
Merged

Use more Yarn workspaces in scripts #7391

merged 13 commits into from
Apr 16, 2025

Conversation

cometkim
Copy link
Member

@cometkim cometkim requested a review from cknitt April 11, 2025 16:34
@cometkim

This comment was marked as resolved.

Sign in to view
@cometkim
Copy link
Member Author

cometkim commented Apr 12, 2025 •
edited
Loading

Hmm, this is weird.

When I inspected the package.tgz, there was no _build directory, so it is just a bug in Yarn's reporting logic.

The protocol error while npm install is even more weird because there are no dependencies at all in the rescirpt package

That was because npm has also a workspaces feature and would try to scan package.json inside of the packages/*

@cometkim cometkim changed the title Add packages to workspaces Use Yarn in the packaging scripts Apr 14, 2025
cometkim
cometkim commented Apr 14, 2025
View reviewed changes
@cometkim cometkim force-pushed the more-workspaces branch 2 times, most recently from ee938f5 to 33999b8 Compare April 14, 2025 19:42
@cometkim cometkim changed the title Use Yarn in the packaging scripts Use more Yarn workspaces in scripts Apr 14, 2025
@cometkim cometkim force-pushed the more-workspaces branch 3 times, most recently from 166c5e5 to 456260f Compare April 14, 2025 22:27
@cometkim
Copy link
Member Author

Not sure how, but binaries-linuxarm64 includes rewatch.exe

@cometkim
Copy link
Member Author

Hmm artifacts -> lib -> build -> rewatch in our Makefile

@cometkim
Copy link
Member Author

cometkim commented Apr 16, 2025 •
edited
Loading

7666f70 fixed the Linux ARM runner to avoid rebuilding rewatch

cknitt
cknitt approved these changes Apr 16, 2025
View reviewed changes
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@cknitt
Copy link
Member

cknitt commented Apr 16, 2025

Oh, there are installation test errors though.

@cometkim
Copy link
Member Author

Re-checked the upload script. It should work.

I think it is better to rewrite some important scripts in JavaScript for easier validation and maintenance.

We could also use a utility like zx.

@cometkim cometkim merged commit 26eeca6 into rescript-lang:master Apr 16, 2025
20 checks passed
@cometkim cometkim deleted the more-workspaces branch April 16, 2025 17:56
cometkim added a commit that referenced this pull request Apr 16, 2025
@cometkim
fix Makefile
72d534a 
`make test` behavior has been changed from #7391 unintendly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cknitt cknitt cknitt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cometkim @cknitt