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

Make AbstractNode-derived docs more specific on RenderObject et al. #130689

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jul 17, 2023

These methods and/or their docs were recently copied (in #128467 and #128973) from their classes' former shared base class AbstractNode. Their wording was fittingly abstract there, but that abstraction is a bit puzzling for a reader finding them on these more concrete classes and not aware of the AbstractNode history. So make them more concrete, in similar terms to the other methods around them.

Also copy some useful points between corresponding methods on different classes (like that the parent of the root is null), and try to clean up the prose on [RenderObject.depth].

We focus on the more outward-facing parts of the API, letting methods like redepthChildren continue to talk generically about "nodes".

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

These methods and/or their docs were recently copied (in flutter#128467
and flutter#128973) from their classes' former shared base class AbstractNode.
Their wording was fittingly abstract there, but that abstraction is a
bit puzzling for a reader finding them on these more concrete classes
and not aware of the AbstractNode history.  So make them more
concrete, in similar terms to the other methods around them.

Also copy some useful points between corresponding methods on
different classes (like that the parent of the root is null),
and try to clean up the prose on [RenderObject.depth].

We focus on the more outward-facing parts of the API, letting
methods like `redepthChildren` continue to talk generically
about "nodes".
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Jul 17, 2023
@gnprice
Copy link
Member Author

gnprice commented Jul 17, 2023

This is a sort of followup to #130688, which is about some of the same docs. Sent as a separate PR because I think that will make both sets of changes simpler to review, because the actual content of the changes is largely unrelated.

Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you for improving the documentation!

@gnprice gnprice added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 17, 2023
@auto-submit auto-submit bot merged commit 1937ae6 into flutter:master Jul 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@gnprice gnprice deleted the pr-tree-docs branch July 17, 2023 21:28
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2023
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
…lutter#130689)

These methods and/or their docs were recently copied (in flutter#128467 and flutter#128973) from their classes' former shared base class AbstractNode. Their wording was fittingly abstract there, but that abstraction is a bit puzzling for a reader finding them on these more concrete classes and not aware of the AbstractNode history.  So make them more concrete, in similar terms to the other methods around them.

Also copy some useful points between corresponding methods on different classes (like that the parent of the root is null), and try to clean up the prose on [RenderObject.depth].

We focus on the more outward-facing parts of the API, letting methods like `redepthChildren` continue to talk generically about "nodes".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants