Skip to content

Change the --include-external flag to be more selective #1289

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

Merged
merged 1 commit into from
Dec 14, 2016
Merged

Change the --include-external flag to be more selective #1289

merged 1 commit into from
Dec 14, 2016

Conversation

keertip
Copy link
Collaborator

@keertip keertip commented Dec 14, 2016

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Dec 14, 2016
@devoncarew
Copy link
Member

(@keertip, you're seeing the same failure as from #1287)

@devoncarew
Copy link
Member

Awesome! lgtm

We when this lands in the sdk, we should rev. the flutter script to use the updated params (and to pull from the dartdoc in the sdk).

@sethladd
Copy link
Contributor

@devoncarew should we open an issue on Flutter side so we don't forget?

@devoncarew
Copy link
Member

👍 sounds good

@keertip keertip merged commit 10e16a2 into dart-lang:master Dec 14, 2016
@keertip keertip deleted the include-external branch December 14, 2016 17:30
@eseidelGoogle
Copy link
Contributor

I happened to notice that Travis was a sad panda, just FYI:
https://travis-ci.org/dart-lang/dartdoc/jobs/183981580#L319

@devoncarew
Copy link
Member

Yup, fixed in #1291.

@@ -252,12 +252,13 @@ class DartDoc {
while (result.hasMoreWork) {
result = context.performAnalysisTask();
}

// Use the includeExternals.
for (Source source in context.librarySources) {
LibraryElement library = context.computeLibraryElement(source);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and libraryName are no longer used in the case where the library doesn't match. No clue if it's expensive or not. (Sorry to keep commenting on your completed CL, was just playing around in this code and noticed.) Thanks again for the fix! :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can always optimize! Though you will see a slowdown once flutter is back to generating all the docs, I saw ~20 sec difference on my machine between the two cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants