(fix) Ensure runtime and napi are emitted as needed.#80
(fix) Ensure runtime and napi are emitted as needed.#80strazzere wants to merge 2 commits intoprebuild:masterfrom
Conversation
Per issue prebuild#79 there seems to be a regression introduced in 7b6dcbd which caused the `target.runtime` to no longer be emitted when a name is used. A name is also _always_ used even if not passed, as it will grab a name from the `package.json`. This commit also dropped including the `napi` tag, which is still utilized downstream. This change fixes the regression and follows this logic; 1. Emit `name` as a tag (this will always be true due to `addName` function logic) 2. Emit `runtime` as a tag always 3. Emit `napi` as a tag if option enabled (likely always true, as it if defaulted to true)
|
Latest node-gyp-build should find it still |
|
Agreed - I only skimmed and did slim testing on it. Just wanted to call it out as I'm not overly familiar with that code. If it seems good to you I have no issues with it - was just trying to go for being overly specific. |
I'm not 100% sure this will, the issue raised above for |
|
This all worked for me locally with node-usb when I submitted the PR.
Though, this was a few months ago so maybe latest has drifted or changed.
To be honest, this entire thing has left my brain as I fixed it locally and
then node-usb reverted back anyway.
I'm unsure if this PR is stale, but I didn't want to hound anyone for
trying to get it merged.
…On Thu, May 2, 2024, 14:13 Rob Moran ***@***.***> wrote:
Latest node-gyp-build should find it still
I'm not 100% sure this will, the issue raised above was using
***@***.*** and ***@***.***
—
Reply to this email directly, view it on GitHub
<#80 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYIRRP536H6U77DM6TK3LZAKT6FAVCNFSM6AAAAABELROUOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJRGYZTEMBZGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Ah! I'm caught up again. @thegecko I think you're misunderstanding the context. The latest The comment you're quoting is in response to my inline comment here;
|
|
I was referring to the comment from @mafintosh
Which suggests this issue is fixed in the latest |
|
Understood, all caught up. You are correct - I believe the comment was meant to be in response to my (then resolved) comments inlined. |
3 participants
Fixes issue #79.
There seems to be a regression introduced in 7b6dcbd which caused the
target.runtimeto no longer be emitted when a name is used. A name is also always used even if not passed, as it will grab a name from thepackage.json. This commit also dropped including thenapitag, which is still utilized downstream.This change fixes the regression and follows this logic;
nameas a tag (this will always be true due toaddNamefunction logic)runtimeas a tag alwaysnapias a tag if option enabled (likely always true, as it if defaulted to true)