-
Notifications
You must be signed in to change notification settings - Fork 29.5k
Standardize dartdoc macro names #69445
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
Standardize dartdoc macro names #69445
Conversation
fdc7f25 to
beabc7c
Compare
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Please make sure to update the style guide accordingly. I share your dubiousness about things like "flutter.widgets.Clip => flutter.material.Material.clipBehavior". I agree that an argument can be made that at least now we know where to go to find the canonical definition. cc @goderbauer |
|
Yes, I'll update the style guide to match. Auditing it might be hard: I'd have to modify dartdoc to spit out the failures, which is a bigger job than just hacking it like I did, since I wouldn't want to impose our rules on others. Or I could build my own script that used the analyzer. Maybe the second option's not too bad, actually. I'll check it out if I have time. |
|
Definitely time box any effort to write automated testing, yeah. What we need is a way to schedule ourselves to do this kind of manual audit regularly. Maybe we should have a Wiki page that says "In October, someone should audit the templates. In May and December, someone should check the deprecation notices for any that need clearing out." along with some helpful hints for how we did each of these in the past. |
|
That's a good idea. I do that about once a year or so with the spell checking, since it's really kind of a manual process. |
|
Can you document the naming style for macros in the style guide (e.g. that they should be named based on the lib they are defined in and based on the property they document?). We should also document in the wiki how to check for this manually every so often. |
goderbauer
left a comment
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
|
Yes, I added a section to the style guide last week. It reads: Dartdoc templates and macrosDartdoc supports creating templates that can be reused in other parts of the code. They are defined /// {@template <id>}
/// ...
/// {@endtemplate}and used via: /// {@macro <id>}The For example: The |
Description
This standardizes the dartdoc template macro names to roughly follow the convention:
flutter.library.Class.member[.optionalDescription]Where there was no need for a description, I didn't add one.
Some (about 1/4) of the macros were already in this form, but standardizing the rest (which were in various forms) resulted in the following transforms:
I think
flutter.widgets.child => flutter.widgets.ProxyWidget.childis kind of dubious, since it's a pretty core thing toWidget, and kind of a detail that it's defined inProxyWidget, but at least you do know where the template resides if you wanted to change it, and makes it easy to find.The slider and range slider changes also might be a step back, but they contain the same information, just in the more standard form.
[And, no, I didn't audit all of the templates by hand: I used a hacked version of dartdoc to print out the canonical names of the symbols each of the templates was attached to, and then (by hand) fixed up the ones that all mapped to the same one by adding their descriptions back in, and made a quick script to do the substitutions in place, verifying first that none of the new names already existed.]
Related Issues
Tests