Skip to content

Check if dartdoc can run on the package. #291

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
Apr 25, 2018

Conversation

isoos
Copy link
Collaborator

@isoos isoos commented Apr 17, 2018

First iteration for #269

@isoos isoos requested a review from jakobr-google April 17, 2018 09:20
@isoos isoos force-pushed the dartdoc_check branch 2 times, most recently from 23c1410 to c242876 Compare April 17, 2018 09:40
@jakobr-google
Copy link
Contributor

Doesn't this mean that we'll be generating the docs both in analyzer and dartdoc instances? If so, can we avoid doing the work twice? Maybe pull the dartdoc result in the analyzer client, and merge it into the analysis data?

@isoos
Copy link
Collaborator Author

isoos commented Apr 25, 2018

Doesn't this mean that we'll be generating the docs both in analyzer and dartdoc instances? If so, can we avoid doing the work twice? Maybe pull the dartdoc result in the analyzer client, and merge it into the analysis data?

Yeah, this is likely the direction we are heading, although there are coordination issues, like: obsolete packages are not analyzed with pana, but are still running with dartdoc. Merging the two processing makes sense, but it requires more work on both pana (configurable/optional items in the analysis) and pub site (merging the two job processing).

For the immediate scope I think the ToolEnvironment parts should go in, and the actual running of dartdoc should be optional. I can split this PR or do a follow-up, whichever you prefer.

Copy link
Contributor

@jakobr-google jakobr-google left a comment

Choose a reason for hiding this comment

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

sgtm, a follow-up PR is fine.

Code LGTM

@isoos isoos merged commit be382f5 into dart-lang:master Apr 25, 2018
@isoos isoos deleted the dartdoc_check branch April 25, 2018 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants