Add viewport documentation breadcrumbs by nscobie · Pull Request #63192 · flutter/flutter

nscobie · 2020-08-07T13:43:57Z

Description

Backfilling documentation breadcrumbs from debugging #61631

Related Issues

N/A

Tests

N/A

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.

flutter-dashboard · 2020-08-07T13:43:59Z

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.

flutter-dashboard · 2020-08-07T13:43:59Z

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.

chunhtai · 2020-08-07T17:30:44Z

packages/flutter/lib/src/rendering/viewport.dart

this comment is a bit confusing, can you explain more?

Tried to clean it up a bit, let me know what you think of the new version. It was meant to mirror the super method's documentation: RenderObject.describeSemanticsConfiguration.

packages/flutter/lib/src/rendering/viewport.dart

packages/flutter/lib/src/rendering/object.dart

goderbauer · 2020-08-12T21:54:08Z

PR triage: @nscobie any chance you can follow up on the comments?

nscobie · 2020-08-14T03:00:08Z

@chunhtai I appreciate the review! I've added a bit more context to each "see also" reference, and tried to clarify the comment about describeSemanticsConfiguration. I don't mind removing anything here that just doesn't seem like helpful detail, but I think these links would have helped me personally when learning how things interact.

@goderbauer apologies for the delay, the start of the semester has given me less time than I'd like to volunteer on this.

chunhtai

These changes are great, they add more explanation to a complicated system.

packages/flutter/lib/src/rendering/object.dart

packages/flutter/lib/src/rendering/viewport.dart

googlebot · 2020-08-14T17:02:24Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

nscobie · 2020-08-14T17:05:09Z

@googlebot I signed it!

googlebot · 2020-08-14T17:05:13Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

chunhtai

LGTM

packages/flutter/lib/src/rendering/viewport.dart

nscobie · 2020-08-14T18:32:13Z

LGTM

👍 Thanks again!

Heads up, I no longer have write access, so you'll need to merge for me (assuming tests pass).

fluttergithubbot · 2020-08-14T19:31:03Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite docs-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

chunhtai · 2020-08-14T19:32:49Z

Hi @nscobie, the doc presubmit check is failing.

nscobie · 2020-08-14T19:55:51Z

Huh, looks like a crash to me. @chunhtai would you mind initiating a re-run?

dartdoc:stdout: Generating docs for library webdriver.sync_io from package:webdriver/sync_io.dart...
dartdoc:stderr: found 0 warnings and 1 error
dartdoc:stdout: Documented 112 public libraries in 730.7 seconds
dartdoc:stderr:
dartdoc:stderr: dartdoc failed: dartdoc encountered 1 errors while processing..
dartdoc:stderr: #0      Dartdoc.generateDocs (package:dartdoc/dartdoc.dart:225:9)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #1      Dartdoc.executeGuarded.<anonymous closure> (package:dartdoc/dartdoc.dart:485:15)
dartdoc:stderr: #2      _rootRun (dart:async/zone.dart:1190:13)
dartdoc:stderr: #3      _CustomZone.run (dart:async/zone.dart:1093:19)
dartdoc:stderr: #4      _runZoned (dart:async/zone.dart:1630:10)
dartdoc:stderr: #5      runZonedGuarded (dart:async/zone.dart:1618:12)
dartdoc:stderr: #6      Dartdoc.executeGuarded (package:dartdoc/dartdoc.dart:483:5)
dartdoc:stderr: #7      main (file:///root/.pub-cache/hosted/pub.dartlang.org/dartdoc-0.32.3/bin/dartdoc.dart:24:11)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #8      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:299:32)
dartdoc:stderr: #9      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)
dartdoc:stderr:
������������

chunhtai · 2020-08-14T19:57:20Z

Hi @nscobie it is not a crash https://api.cirrus-ci.com/v1/task/5909429256192000/logs/main.log

dartdoc:stdout: Generating docs for library material from package:flutter/material.dart...
dartdoc:stdout: Generating docs for library painting from package:flutter/painting.dart...
dartdoc:stdout: Generating docs for library physics from package:flutter/physics.dart...
dartdoc:stdout: Generating docs for library rendering from package:flutter/rendering.dart...
dartdoc:stderr:   error: unresolved doc reference [SingleChildScrollView.showOnScreen], from rendering.RenderViewportBase.showInViewport: (file:///tmp/flutter%20sdk/packages/flutter/lib/src/rendering/viewport.dart:1064:15)
dartdoc:stdout: Generating docs for library scheduler from package:flutter/scheduler.dart...
dartdoc:stdout: Generating docs for library semantics from package:flutter/semantics.dart...

nscobie · 2020-08-14T20:08:13Z

Ah thanks for pointing that out, I should have looked at the full log.

It's because I overlooked that SingleChildScrollView actually relies on _RenderSingleChildViewport to override showOnScreen. I'll update the docs to reflect that.

nscobie · 2020-08-14T22:01:58Z

CI looks happy now 🎉

nscobie requested a review from gaaclarke August 7, 2020 13:43

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 7, 2020

nscobie mentioned this pull request Aug 7, 2020

Minor documentation details/breadcrumbs flutter/engine#20330

Merged

8 tasks

nscobie force-pushed the breadcrumbs branch from 6434ccc to d9940b8 Compare August 7, 2020 13:54

chunhtai reviewed Aug 7, 2020

View reviewed changes

nscobie force-pushed the breadcrumbs branch from d9940b8 to 027f140 Compare August 14, 2020 02:49

googlebot added the cla: yes label Aug 14, 2020

chunhtai reviewed Aug 14, 2020

View reviewed changes

nscobie force-pushed the breadcrumbs branch from 027f140 to 775c18c Compare August 14, 2020 17:02

googlebot added cla: no and removed cla: yes labels Aug 14, 2020

googlebot added cla: yes and removed cla: no labels Aug 14, 2020

chunhtai approved these changes Aug 14, 2020

View reviewed changes

packages/flutter/lib/src/rendering/viewport.dart Outdated Show resolved Hide resolved

nscobie force-pushed the breadcrumbs branch from 775c18c to eda881d Compare August 14, 2020 18:29

chunhtai added the waiting for tree to go green label Aug 14, 2020

fluttergithubbot removed the waiting for tree to go green label Aug 14, 2020

Add viewport documentation breadcrumbs

98a8cbf

nscobie force-pushed the breadcrumbs branch from eda881d to 98a8cbf Compare August 14, 2020 20:09

goderbauer added the waiting for tree to go green label Aug 14, 2020

fluttergithubbot merged commit ede1b15 into flutter:master Aug 14, 2020

smadey pushed a commit to smadey/flutter that referenced this pull request Aug 27, 2020

Add viewport documentation breadcrumbs (flutter#63192)

953f2e4

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Add viewport documentation breadcrumbs (flutter#63192)

25f967d

github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021

Conversation

nscobie commented Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

flutter-dashboard bot commented Aug 7, 2020

Uh oh!

flutter-dashboard bot commented Aug 7, 2020

Uh oh!

chunhtai Aug 7, 2020

Choose a reason for hiding this comment

Uh oh!

nscobie Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goderbauer commented Aug 12, 2020

Uh oh!

nscobie commented Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

googlebot commented Aug 14, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

nscobie commented Aug 14, 2020

Uh oh!

googlebot commented Aug 14, 2020

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nscobie commented Aug 14, 2020

Uh oh!

fluttergithubbot commented Aug 14, 2020

Uh oh!

chunhtai commented Aug 14, 2020

Uh oh!

nscobie commented Aug 14, 2020

Uh oh!

chunhtai commented Aug 14, 2020

Uh oh!

nscobie commented Aug 14, 2020

Uh oh!

nscobie commented Aug 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nscobie commented Aug 7, 2020 •

edited

Loading

nscobie Aug 14, 2020 •

edited

Loading

nscobie commented Aug 14, 2020 •

edited

Loading