-
Notifications
You must be signed in to change notification settings - Fork 159
Run pkg/pub_dartdoc instead of global dartdoc #1384
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
Conversation
Staging looks good, |
This is not flawless in staging, e.g. sometimes we get this warning from dartdoc and it breaks the further processing: |
Looks like that warning does not affect the results, only it gets more visible when there are no .dart files in the package, but the log warning is about not having files there, the warning is unrelated. |
Oh, merging... after it gets reviewed :) |
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.
lgtm
Whoops forgot to submit my review.
final hasIndexJson = | ||
await new File(p.join(outputDir, 'index.json')).exists(); | ||
return new DartdocResult( | ||
pr, pr.exitCode == 15, hasIndexHtml, hasIndexJson); |
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.
What about other error codes besides 0 and 15?
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.
15 is SIGTERM, which we get when the timeout kills the process. In that case we retry it without link validation, which is a time-consuming thing to do on a large archive. On any non-zero we emit the the log.warning about it.
However we don't really care about the exit code besides these two, as long as the run produced index.html
and index.json
(and we may add pub-data.json
to that list too).
#1179
I'll deploy this to staging with the initialization PR to test it.