build: account for pure C sources in build-addons-napi
#21797
Conversation
addaleax
added
build
nodejs-github-bot
added
the
build
|
CI: https://ci.nodejs.org/job/node-test-pull-request/15861/ Feel free to 👍 this comment to approve fast-tracking. |
|
/CC @nodejs/build-files Why is there a need for fast track? |
|
@refack Why does there have to be a need? It makes sense to me… It’s a simple change – a one-liner in the most literal way – so it’s not like one could reasonably expect a lot of review on it. It also fixes a real issue (that some N-API tests don’t get recompiled if they are changed, which is a bad thing), but that’s not really my point here… |
addaleax
removed
the
fast-track
The 48h-72h time is not only for review, it's also a chance for Collaborators to become aware of changes. There's is a general notion of areas of expertise and code ownership, so IMHO the stakeholder should have a chance to be aware of changes. I agree it's a simple change, but it's not trivial (changes the build/test recipe), nor does it fix a regression (AFAICT). I'd rather take it slow, especially since the last change in those lines was regressive. 57bd27e |
|
Landed in 31ecf63 |
PR-URL: nodejs#21797 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #21797 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes