-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: only run build-windows
workflow on source changes
#51596
tools: only run build-windows
workflow on source changes
#51596
Conversation
Review requested:
|
4c4d0cf
to
ee46d05
Compare
FWIW there is an open issue on running the tests as well for Windows on the GitHub runners: #50489 |
Landed in 4ab63db |
PR-URL: nodejs#51596 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
PR-URL: #51596 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
PR-URL: nodejs#51596 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
This prevents |
It also doesn't run when deps are updated: #51362 |
Those sound like good reasons to revert this. |
PR-URL: #51596 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
PR-URL: #51596 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Since this workflow does not run any tests, it doesn't really makes sense to have it run on doc-only changes, or test-only changes.
I took inspiration for the list of paths from
node/.github/workflows/coverage-windows.yml
Lines 6 to 14 in 084d761