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

Pana may wish to consider docs coverage in the score (or at least list as a warning) #269

Closed
eseidelGoogle opened this issue Mar 16, 2018 · 14 comments

Comments

@eseidelGoogle
Copy link

Flutter for example tracks that all public members in flutter/flutter are documented.

flutter analyze --flutter-repo --dartdocs is the command we use.

Which just turns on:
http://dart-lang.github.io/linter/lints/package_api_docs.html

@isoos
Copy link
Collaborator

isoos commented Mar 16, 2018

I think this could be part of the code health (vs. the other option is maintenance).

Question: given a package with great documentation with 100/100 health score, if you'd remove all of the API docs, how much would you decrease its score? What would be a "fair" percentage for it?

@kwalrath
Copy link

90/100? There's a tradeoff between encouraging doc comments and people gaming the system by writing unhelpful doc comments just to increase the score.

@isoos
Copy link
Collaborator

isoos commented Mar 20, 2018

For the first iteration, we could do the following

  • Run dartdoc.
  • After a successful dartdoc run, we'll get an index.json, that contains all the public entry points of the API. Counting them gives us the apiCount.
  • When we run dartanalyzer, we can turn on the public_member_api_docs linter rule. The number of the reported issues gives us the missingDocCount.
  • health (Fitness) needs to be adjusted with these, although there is already an "api size estimate" in the current Fitness.magnitude.

This does not handle empty documentation, nor does it take the method size into account, e.g. the following will be counted as one missing API entry point out of the many...

///
void method() {}

@kevmoo
Copy link
Member

kevmoo commented Apr 17, 2018

FYI: public_member_api_docs seems to be (very) broken - dart-lang/sdk#57310

@kwalrath
Copy link

kwalrath commented Apr 17, 2018

How hard should we be on packages that don't rely on dartdoc-generated docs as their primary reference docs?

I'm thinking of angular_components, which relies on the gallery at https://dart-lang.github.io/angular_components_example rather than API docs, since you usually use the components via HTML rather than Dart.

I suppose we can detect these packages by whether they have a non-blank documentation field in their pubspec.

/cc @nshahan @TedSander

@kwalrath
Copy link

kwalrath commented Apr 18, 2018

Thinking about this some more, there's a hierarchy of doc importance:

  1. README (used for API doc homepage; crucially important; has multiple audiences: API doc readers, package page readers, github site visitors)
  2. library description (every library should have one, maybe even if the library isn't included in API docs by default)
  3. class description (every important class should have one; ideally, all classes should give or point to a little context about when you might need the class and what other classes you should consider instead)
  4. member descriptions (not always necessary; better to have no description than boilerplate)

@nshahan
Copy link

nshahan commented Apr 19, 2018

I see a lot of instances where a library contains a single class. Typically there isn't a library doc. All the documentation is on the class. @kwalrath WDYT?

@kwalrath
Copy link

@nshahan I can see how that might be reasonable for packages that have tons of very small libraries, where the API isn't primarily Dart. It's not a great experience for people who happen to find the package or library docs, though.

@isoos
Copy link
Collaborator

isoos commented May 30, 2018

I have a very early-version PoC of using package:dartdoc to extract the documentation text (thanks @jcollins-g for the help). We could use the same thing for various purposes: to indicate in the analysis if a package is under-documented, and also to include it in the search when documentation is present.

The catch is: the analysis part (building the PackageGraph in dartdoc) takes over a minute for a package like pana, and it is repeated when we are generating docs for the package. This would mean that the average package analysis time would be 3-4x the current times.

The balancing act here is that we want the analysis upgrades to be fast (e.g. when there is a version / data schema change), and it needs to be figured out how this can be done on the pub site's scale. (/cc @jakobr-google)

@kevmoo
Copy link
Member

kevmoo commented May 31, 2018

Maybe we have another score that is the doc score and we separate that out from pana.

Pana is relatively straight-forward now. Adding dartdoc as an integration point would add a lot of complexity.

One could argue dartdoc could "learn" how to give a score when it generates docs...

@isoos
Copy link
Collaborator

isoos commented May 31, 2018

Good points! There is also the potential that we can introduce new scoring segments in the near future, so it would be worth to re-think how we shall store and update the scores.

@isoos
Copy link
Collaborator

isoos commented Jun 6, 2018

Follow-up: I'm implementing a data extraction from package:dartdoc in dart-lang/pub-dev#1353. This will be the basis of the dartdoc coverage score (and also the input for search), but it won't be through pana.

@kevmoo
Copy link
Member

kevmoo commented Jun 6, 2018 via email

@isoos
Copy link
Collaborator

isoos commented Aug 26, 2020

This has been implemented for a while now.

@isoos isoos closed this as completed Aug 26, 2020
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

No branches or pull requests

5 participants