build: don't require processing docs for nightlies #8325

Merged
merged 1 commit into from Sep 1, 2016

Conversation

Projects
None yet
7 participants
@jbergstroem
Member

jbergstroem commented Aug 29, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected 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

@jbergstroem

This comment has been minimized.

Show comment
Hide comment
@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Aug 29, 2016

Member

LGTM

Member

addaleax commented Aug 29, 2016

LGTM

@jbergstroem jbergstroem referenced this pull request in nodejs/build Aug 29, 2016

Closed

Linux builds missing from nightly folders #478

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 29, 2016

Member

LGTM

Member

jasnell commented Aug 29, 2016

LGTM

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Aug 29, 2016

Member

LGTM

Member

mhdawson commented Aug 29, 2016

LGTM

@bnoordhuis

View changes

Makefile
@@ -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 \

This comment has been minimized.

@bnoordhuis

bnoordhuis Aug 30, 2016

Member

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.)

@bnoordhuis

bnoordhuis Aug 30, 2016

Member

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.)

This comment has been minimized.

@jbergstroem

jbergstroem Aug 30, 2016

Member

Happy to change -- I agree with opt-out but please elaborate on a use case when custom would benefit from the check!

@jbergstroem

jbergstroem Aug 30, 2016

Member

Happy to change -- I agree with opt-out but please elaborate on a use case when custom would benefit from the check!

This comment has been minimized.

@bnoordhuis

bnoordhuis Aug 30, 2016

Member

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.

@bnoordhuis

bnoordhuis Aug 30, 2016

Member

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.

This comment has been minimized.

@jbergstroem

jbergstroem Aug 30, 2016

Member

Ok. I'll make a note in the commit msg.

@jbergstroem

jbergstroem Aug 30, 2016

Member

Ok. I'll make a note in the commit msg.

@jbergstroem

This comment has been minimized.

Show comment
Hide comment
@jbergstroem

This comment has been minimized.

Show comment
Hide comment
@jbergstroem

jbergstroem Aug 31, 2016

Member

Failures unrelated. I've tested with DISTTYPE=release and DISTTYPE=custom locally. Will rebase and squash/merge today unless there are any new feedback.

Member

jbergstroem commented Aug 31, 2016

Failures unrelated. I've tested with DISTTYPE=release and DISTTYPE=custom locally. Will rebase and squash/merge today unless there are any new feedback.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Sep 1, 2016

Member

LGTM if you add a line or two to the commit log. Right now it's just a one-liner.

Member

bnoordhuis commented Sep 1, 2016

LGTM if you add a line or two to the commit log. Right now it's just a one-liner.

build: don't require processing docs for nightlies
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

This comment has been minimized.

Show comment
Hide comment
Member

jbergstroem commented Sep 1, 2016

@jbergstroem jbergstroem merged commit 2168432 into nodejs:master Sep 1, 2016

@Fishrock123 Fishrock123 referenced this pull request Sep 6, 2016

Closed

v6.6.0 pre-proposal #8428

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

build: don't require processing docs for nightlies
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>

Fishrock123 added a commit that referenced this pull request Sep 9, 2016

build: don't require processing docs for nightlies
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>
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Sep 30, 2016

Member

@jbergstroem I've backported to v4.x-staging. Let me know if it shouldn't have been

Member

MylesBorins commented Sep 30, 2016

@jbergstroem I've backported to v4.x-staging. Let me know if it shouldn't have been

MylesBorins added a commit that referenced this pull request Sep 30, 2016

build: don't require processing docs for nightlies
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

This comment has been minimized.

Show comment
Hide comment
Member

jbergstroem commented Oct 2, 2016

MylesBorins added a commit that referenced this pull request Oct 10, 2016

build: don't require processing docs for nightlies
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>

rvagg added a commit that referenced this pull request Oct 18, 2016

build: don't require processing docs for nightlies
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>

MylesBorins added a commit that referenced this pull request Oct 26, 2016

build: don't require processing docs for nightlies
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>

@MylesBorins MylesBorins referenced this pull request Oct 26, 2016

Closed

V4.6.2 proposal #9298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment