Skip to content

Solution for pub's Health issues and suggestions #1881

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

Closed
hyochan opened this issue Dec 16, 2018 · 8 comments
Closed

Solution for pub's Health issues and suggestions #1881

hyochan opened this issue Dec 16, 2018 · 8 comments

Comments

@hyochan
Copy link

hyochan commented Dec 16, 2018

image
Recently, I'm seeing the `Document public APIs issue in pub page. How can I solve this -9.86 points?

Our plugin in is flutter_calendar_carousel.

@jonasfj
Copy link
Member

jonasfj commented Dec 17, 2018

The obvious solution is to document the public API. But I would agree that it might be a little extreme to document all the default<...>TextStyle values.

@hyochan, is there some way we could improve this message, so it easier to understand what it suggests?

@hyochan, unrelated, reading your code gave me an idea for a new analysis rule: dart-lang/pana#461 (forgetting to remove print() is so common, I can't count the number of times it's gone wrong for me).

@isoos, is it required that all public members are documented? When looking at the source I can see that there is a lot of trivial members, and documenting them seems plain overkill: https://github.com/dooboolab/flutter_calendar_carousel/blob/825df5e8c1e5224972776efc1ed5e0047fa6cf04/lib/flutter_calendar_carousel.dart#L10

Also would it help if in the suggest we showed something a long the lines of :

/// <missing API documentation>
class MyClass {

@isoos
Copy link
Collaborator

isoos commented Dec 17, 2018

is it required that all public members are documented?

Right now we show that setting if the documented API members are <20%.

When looking at the source I can see that there is a lot of trivial members, and documenting them seems plain overkill

One could argue that trivial for one developer may not be trivial to another.

Having said that, in this case it seems that using full English words and CamelCase names should be a good-enough substitute for documentation.

@hyochan
Copy link
Author

hyochan commented Dec 17, 2018

@jonasfj I've written about defaultValues in Props table which seems not sufficient in pub. I am still confused. What is the solution currently to fix -9.86 points? What I've got here are

  1. listing contributors
  2. removing print message
  3. writing up defaultValues separately.
    Are these correct? Am I understanding correctly?

@isoos
Copy link
Collaborator

isoos commented Dec 17, 2018

What is the solution currently to fix -9.86 points?

You need to put dartdoc on fields (or at least 20% of them), e.g.

  /// The border color of the previous day of the month.
  final Color prevMonthDayBorderColor;

Or, you could ignore it for a while, because (likely in January) we could create a new heuristic, that would accept field names like the above without documentation (because they seem to be descriptive enough).

I would suggest that you just ignore it for a while (especially because the package is popular and otherwise clean of obvious issues).

@hyochan
Copy link
Author

hyochan commented Dec 17, 2018

Thanks. I'll come back 2019. 🥺

@isoos
Copy link
Collaborator

isoos commented Jan 3, 2019

@hyochan: The bad news is: after a bit of experimentation, we have postponed our plans to automatically detect 'self-describing' API symbols. While many of your getters and methods are descriptive, we will consider only proper /// comments as valid documentation.

The good news part is that we are decreasing the penalty, and in your case, it won't be -9.86, rather around -0.7 (in a near-future release). To eliminate it, please document a few more API members (methods, fields, classes).

Note: our aim is to encourage people to document their package, and in the future we may slowly increase the thresholds for penalties.

@isoos isoos closed this as completed Jan 3, 2019
@hyochan
Copy link
Author

hyochan commented Jan 6, 2019

@isoos Well in the readme, I've already described almost all the props. Since, this is a widget and not a plugin, I want to know how can I describe more. Thanks for your update!

@isoos
Copy link
Collaborator

isoos commented Jan 11, 2019

@hyochan at the moment we are not able and not planning to parse README on the level to detect your props table. We rely on documentation comments, e.g. in the following snippet, I've added lines with three slash characters (///):

/// Documentation for the class.
class CalendarCarousel extends StatefulWidget {
  /// Documentation for this field.
  final TextStyle defaultHeaderTextStyle = TextStyle(
    fontSize: 20.0,
    color: Colors.blue,
  );

  /// Documentation for this field.
  final double viewportFraction;

  /// Documentation for this field.
  final TextStyle prevDaysTextStyle;

  /// Documentation for this field.
  final TextStyle daysTextStyle;

  /// Documentation for this field.
  final TextStyle nextDaysTextStyle;

Text content that gets there will also be presented in locations like the documentation here: class or field and the pub site search will use that content for search too.

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

3 participants