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

Sort file lists in artifacts.json #5474

Merged
merged 1 commit into from
Jun 26, 2022
Merged

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Jun 26, 2022

scripts/prepublish.js is (as far as I understand) meant to be run in the local working copy before publishing to npm.

It

  • runs npm pack --dry-run to get the list of files that would be published
  • transforms the list into a JSON structure (why?)
  • writes that structure to jscomp/artifacts.json and
  • diffs that file against what's currently committed in the (remote) repo.

This way the person doing npm publishing can see if all expected files are there (e.g., binaries for the different platforms) and if there are any new or removed files since the last publish.

I ran the script on my Mac and got a huge amount of changes, because the files were returned in a different sort order than in the current artifacts.json.

I have therefore sorted the current artifacts.json manually (actually with a small helper script) and also changed prepublish.js to sort the files. Now when I run prepublish.js again, I get an actually usable and useful diff of "current" vs. "expected".

Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Good catch.

@cknitt cknitt merged commit 9af349c into rescript-lang:master Jun 26, 2022
@cknitt cknitt deleted the sort-artifacts branch June 26, 2022 19:13
mununki pushed a commit to mununki/rescript-compiler that referenced this pull request Jul 15, 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.

2 participants