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

fix: node.js publish #502

Merged
merged 6 commits into from
Aug 26, 2024
Merged

fix: node.js publish #502

merged 6 commits into from
Aug 26, 2024

Conversation

matthewkeil
Copy link
Member

@matthewkeil matthewkeil commented Aug 26, 2024

When publishing 4.0.0 there was an error and the bundle published without the deps folder. The make command did not correctly switch directories and published the root folder, not the dist folder. I updated the process to use the canonical method that node uses to avoid future issues.

I also updated the supported/tested versions of node that are run in CI

@@ -1,10 +1,10 @@
{
"name": "c-kzg",
"version": "4.0.0",
"version": "4.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

Found the issue after running the publish command with 4.0.0 so needed to bump the minor. Not sure why this showed up now though.

Comment on lines -47 to -56
# Bundle the distribution, also helpful for cross compatibility checks
.PHONY: bundle
bundle: build
@mkdir dist
@mv deps dist
@cp -r lib dist/lib
@cp -r src dist/src
@cp README.md dist/README.md
@cp package.json dist/package.json
@cp binding.gyp dist/binding.gyp
Copy link
Member Author

Choose a reason for hiding this comment

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

The files key in the package.json is what controls this behavior. There is no need to build the dist folder. Went with this before because you guys seemed to like it so rode the wave but the error showed up so switched this to the canonical method of using the files key to control what gets bundled in the npm tarball

Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@jtraglia jtraglia merged commit 85a964e into ethereum:main Aug 26, 2024
41 checks passed
@jtraglia jtraglia deleted the mkeil/fix-publish branch August 26, 2024 18:22
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