build: only require docs 'Added' fixes for release#12958
build: only require docs 'Added' fixes for release#12958rvagg wants to merge 1 commit intonodejs:masterfrom
Conversation
Makefile
Outdated
There was a problem hiding this comment.
tiniest of style nits, but I think we tend to use = rather than ==
|
This was part of a review by @bnoordhuis in #8325 (comment) . I'd be happy to see this check gone for test builds, I've been floating patches since this was introduced. However, this also affects RCs, and for those this should probably still be in place (cc @nodejs/release). Both use |
|
What about this, to check RCs but not test builds: JaneaSystems@e8573c4 ? @rvagg feel free to take it, discuss, or I can open another PR if you prefer. |
d1d1ada to
6aa5896
Compare
|
@jasnell you've done most of the leading-edge RC builds recently, what do you think about locking in need to do the @nodejs/build I've put this into the release Jenkins as a quick workaround for now as I'm messing with these new v8-canary builds and they are impacted too. @joaocgreis it's more than just "test" that impacts it, in fact I've been refactoring the Jenkins parameters and scripts to make even more use of "custom" disttype to do v8-canary as well as test and rc. So IMO we should be exclusive rather than inclusive in the requirements for if [[ "X${disttype}" != "Xrelease" ]]; then
if [[ "X${disttype}" == "Xcustom" ]]; then
replaceme_version="${CUSTOMTAG}"
else
replaceme_version="${disttype}${datestring}${commit}"
fi
perl -pi -e "s/REPLACEME/${replaceme_version}/g" doc/api/*.md
fiPerhaps we should even consider doing this for non release builds in the Makefile so you don't ever build docs with |
|
Actually I'm just going to go with this in Jenkins for a quick workaround until this is solved: if [[ "X${disttype}" != "Xrelease" ]]; then
perl -pi -e "s/: release-only/:/g" Makefile
fi |
|
@rvagg where do we stand here? Should this land or do you want to change some more things? |
|
don't close, I'm on it |
|
To be clear, if the release team doesn't have an issue with RCs not getting the warning (and @jasnell already approved so I assume that's the case), then I have no objection. |
|
Ping @rvagg |
|
Closing due to long inactivity. @rvagg please reopen if you would like to further pursue this. |
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>
Discovered while trying to do a
testbuild for #12957. There areAdded:tags without versions in the docs onmasterso it's borking on build. Test builds use theDISTTYPEof"custom"which isn't allowed through here, like"nightly"is. So I've inverted the logic since"release"would be the onlyDISTTYPE(also the default if you don't set one) that's allowed through./cc @nodejs/build