Skip to content
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

N-API documentation appears on 7.10.0 #12833

Closed
gabrielschulhof opened this issue May 4, 2017 · 13 comments
Closed

N-API documentation appears on 7.10.0 #12833

gabrielschulhof opened this issue May 4, 2017 · 13 comments
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. question Issues that look for answers.

Comments

@gabrielschulhof
Copy link
Contributor

https://nodejs.org/dist/latest-v7.x/docs/api/n-api.html

N-API was not supposed to land on v7.x, right?

@mscdex mscdex added node-api Issues and PRs related to the Node-API. question Issues that look for answers. doc Issues and PRs related to the documentations. labels May 4, 2017
@mscdex
Copy link
Contributor

mscdex commented May 4, 2017

/cc @nodejs/n-api

@addaleax
Copy link
Member

addaleax commented May 4, 2017

Yes, this is weird, the file isn’t even there in the v7.10.0 tag. @nodejs/build ideas?

@mhdawson
Copy link
Member

To double check I checked out v7.x and build the docs. No n-api.

@mhdawson
Copy link
Member

Does not show for the 6.x docs, so seems specific to 7.x

@mhdawson
Copy link
Member

There seems to be 2 copies in the 7.10.0 and 7.x directories. The second which is one level down from the first (https://nodejs.org/dist/v7.10.0/docs/doc/api) does not have N-API.

@mhdawson
Copy link
Member

I've asked in the build IRC channel who knows how the docs are built for the website to see if they can point me in the right direction.

@rvagg
Copy link
Member

rvagg commented May 11, 2017

looking in to it

@rvagg
Copy link
Member

rvagg commented May 11, 2017

These other versions have both a docs/api and docs/doc/api:

v4.0.0
v4.1.0
v4.2.0
v4.4.0
v4.6.1
v4.7.0
v4.7.3
v4.8.0
v4.8.2
v4.8.3
v5.1.0
v5.2.0
v5.3.0
v5.9.1
v6.0.0
v6.10.0
v6.2.1
v6.2.2
v6.4.0
v6.5.0
v6.9.1
v6.9.2
v6.9.5
v7.10.0
v7.1.0
v7.2.0
v7.8.0

Seems random, must be something to do with our build process (which is all in Makefile btw, called from make doc-upload on the build system).

So far in my checking, only 7.10.0 seem to have different content and that difference is only the addition of the n-api docs. Very strange!

Going deeper, will let you know when I find something, if you don't hear from me soon, send help!

@evanlucas
Copy link
Contributor

Yea, I had actually cherry-picked it by accident initially, but pulled it back out when I cut the release. Sorry if I caused this

@rvagg
Copy link
Member

rvagg commented May 11, 2017

@evanlucas how close to the release? and did you force-push? it's possible that these double directories are from dirty directories being reused but I'm still scratching my head how the rest are identical but this one is different.

@rvagg
Copy link
Member

rvagg commented May 11, 2017

Got it! you get the double directory if CI rebuilds the docs before its promoted, there's an scp -pr out/doc/ server:distdir/docs/ in there and if it's run twice then you end up with the expected directory and the second time gives you a doc subdirectory with the mirror.

So my assumption here is that 7.10.0 was first built with the n-api commit in it, then was rebuilt without it, so the docs that come from the release commit are in docs/doc/api/ and the docs that that are from the abandoned commit that was first built are in docs/api/.

I'll come up with a fix for Makefile and PR that shortly.

I'll also manually sort out that 7.10.0 directory issue by nuking docs/api/ and moving docs/doc/api/ up to docs/api/ - @nodejs/build can I get a +1 on doing that?

@rvagg
Copy link
Member

rvagg commented May 11, 2017

#12957 is the fix

@evanlucas
Copy link
Contributor

@rvagg yay! Thanks for figuring this out. Apologies for causing the problem, although I guess it is good that we know about it now :]

jasnell pushed a commit that referenced this issue May 24, 2017
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
jasnell pushed a commit that referenced this issue May 28, 2017
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 17, 2017
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 17, 2017
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 25, 2017
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

6 participants