-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
build: don't require processing docs for nightlies #8325
Conversation
LGTM |
LGTM |
1 similar comment
LGTM |
@@ -460,7 +460,7 @@ PACKAGEMAKER ?= /Developer/Applications/Utilities/PackageMaker.app/Contents/MacO | |||
PKGDIR=out/dist-osx | |||
|
|||
release-only: | |||
@if `grep -q REPLACEME doc/api/*.md`; then \ | |||
@if [ "$(DISTTYPE)" = "release" ] && [ `grep -q REPLACEME doc/api/*.md` ]; then \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to check that DISTTYPE is not nightly or next-nightly. If it's set to "custom", you presumably still want this check.
(Also, opting out is arguably better than opting in when it comes to quality checks.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to change -- I agree with opt-out but please elaborate on a use case when custom would benefit from the check!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's based on the assumption that people use it for internal releases. We do that at IBM although I don't know if we use this particular mechanism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'll make a note in the commit msg.
Failures unrelated. I've tested with |
LGTM if you add a line or two to the commit log. Right now it's just a one-liner. |
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: nodejs#8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
a186944
to
2168432
Compare
Commit message validation: https://ci.nodejs.org/job/node-test-commitmsg/16/tapResults/ |
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: nodejs#8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: #8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jbergstroem I've backported to v4.x-staging. Let me know if it shouldn't have been |
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: #8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@thealphanerd sgtm. |
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: #8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: #8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Opt-out `nightly` and `next-nightly` from the documentation requirement since these docs aren't meant to be published. This fixes our nightly jobs in CI. PR-URL: #8325 Fixes: nodejs/build#478 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#24575 Refs: nodejs#24551 Refs: nodejs#12958 Refs: nodejs#12957 Refs: nodejs#8325 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
build
Description of change
Our nightlies were experiencing issues since we didn't properly replace version info in documentation (#6864). Skip that check unless we're actually doing release builds.
/cc @mhdawson, @williamkapke (reporter), @bnoordhuis, @addaleax