Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
build: only require docs 'Added' fixes for release#12958
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Makefile Outdated
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.
tiniest of style nits, but I think we tend to use = rather than ==
joaocgreis commented May 11, 2017
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 |
joaocgreis commented May 28, 2017
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 6aa5896Comparervagg commented Jun 9, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@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" ]];thenif [[ "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 |
rvagg commented Jun 9, 2017
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 |
BridgeAR commented Aug 26, 2017
@rvagg where do we stand here? Should this land or do you want to change some more things? |
BridgeAR commented Sep 12, 2017
rvagg commented Sep 13, 2017
don't close, I'm on it |
joaocgreis commented Sep 13, 2017
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. |
BridgeAR commented Sep 22, 2017
Ping @rvagg |
BridgeAR commented Oct 2, 2017
Closing due to long inactivity. @rvagg please reopen if you would like to further pursue this. |
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#24575 Refs: nodejs#24551 Refs: nodejs#12958 Refs: nodejs#12957 Refs: nodejs#8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24575 Refs: #24551 Refs: #12958 Refs: #12957 Refs: #8325 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Richard Lau <[email protected]>
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