Skip to content

Conversation

@miafan23
Copy link
Contributor

@miafan23 miafan23 commented Jul 3, 2025

  • Move prebuildify to dev dependencies to avoid introducing vulnerabilities to other apps
  • Fix CI

@miafan23 miafan23 changed the title Move prebuildify to dev Move prebuildify to dev dependencies Jul 3, 2025
@miafan23 miafan23 force-pushed the move-prebuildify-to-dev branch from f207d74 to 11e3ac4 Compare July 4, 2025 10:55
@miafan23 miafan23 marked this pull request as ready for review July 4, 2025 11:16
@miafan23 miafan23 requested a review from AlexanderBelokon July 4, 2025 11:16
"test": "./node_modules/.bin/tape test/**/*.test.js",
"prebuildify": "prebuildify --napi --tag-uv --tag-libc --strip"
"prebuildify": "prebuildify --napi --tag-uv --tag-libc --strip",
"prepublishOnly": "npm run download-binaries"
Copy link
Contributor

@AlexanderBelokon AlexanderBelokon Jul 4, 2025

Choose a reason for hiding this comment

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

I'd remove prebuilds first, and after download-binaries throw an error if one of the platforms is missing, otherwise it's easy to publish binaries from a previous release.

Also, can we check that binaries are built from the current gitsha? Like maybe add the gitsha to artifact name or something?

This whole process feels very brittle right now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, can we check that binaries are built from the current gitsha? Like maybe add the gitsha to artifact name or something?

When the CI is not done on my new push, I am not able to download binaries, so I guess it works like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexanderBelokon Thanks for the review. Updated the script ed67262

miafan23 and others added 2 commits July 4, 2025 14:48
Co-authored-by: SanD <alexander@belokon.net>
Copy link
Contributor

@AlexanderBelokon AlexanderBelokon left a comment

Choose a reason for hiding this comment

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

Great work!

@miafan23 miafan23 merged commit 69c32a0 into master Jul 4, 2025
12 checks passed
greatBlueHerron pushed a commit that referenced this pull request Aug 27, 2025
Co-authored-by: AlexanderBelokon <alexander.belokon@mapbox.com>
Co-authored-by: SanD <alexander@belokon.net>
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