ref(profiling-node)!: Remove prune-profiler-binaries bin script - #23104
Merged
Conversation
Removes the `sentry-prune-profiler-binaries` bin script and its `bin`/`files` entries from `@sentry/profiling-node`, along with its test and the related docs. Fixes #20567 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
nicohrubec
marked this pull request as ready for review
August 6, 2026 16:04
nicohrubec
requested review from
JPeer264,
andreiborza,
chargome and
mydea
and removed request for
a team and
mydea
August 6, 2026 16:04
| Sentry: pruned ./dist/sentry_cpu_profiler-win32-x64-93-3PKQDSGE.node (181.50 KiB) | ||
| ✅ Sentry: pruned 15 binaries, saved 1.06 MiB in total. | ||
| ``` | ||
| runtime. Since the binaries follow the `sentry_cpu_profiler-<platform>-<arch>-<stdlib>-<abi>.node` naming scheme, you can |
Contributor
There was a problem hiding this comment.
Bug: The documentation incorrectly describes the binary naming scheme, stating all platforms include a <stdlib> segment, which is only true for Linux.
Severity: LOW
Suggested Fix
Update the documentation to accurately reflect the different naming schemes for each platform (Linux, Darwin, Windows). Specifically, clarify that the <stdlib> segment is Linux-specific. Also, include the <hash> suffix in the example patterns to match the real filenames. For example, provide separate patterns for Linux (...-<stdlib>-...), Darwin (...-<arch>-<abi>...), and Windows.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/profiling-node/README.md#L237
Potential issue: The documentation at `packages/profiling-node/README.md:237` introduces
an inaccurate description for the profiler's binary naming scheme. It presents the
pattern `sentry_cpu_profiler-<platform>-<arch>-<stdlib>-<abi>.node` as universal.
However, the `<stdlib>` segment is only present for Linux platforms (e.g., `glibc`,
`musl`). For macOS (Darwin) and Windows, this segment is absent. This inaccuracy will
cause confusion for developers following the new manual instructions for pruning
binaries, as they will be unable to find files matching the documented pattern on
non-Linux systems. The documentation also omits the `<hash>` suffix present in actual
filenames.
Did we get this right? 👍 / 👎 to inform future reviews.
andreiborza
approved these changes
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the
sentry-prune-profiler-binariesbin script from@sentry/profiling-node.The script was already deprecated with a runtime warning and a README notice pointing at #20567. This PR follows through on that removal:
scripts/prune-profiler-binaries.jsand its test.binmapping and the script'sfilesentry frompackage.json.docs/supporting-a-new-node-version.md, since theNODE_TO_ABImap no longer needs maintaining.The MIGRATION.md entry for this removal already exists on
develop, so it is unchanged here.Fixes #20567