Skip to content

More restoration documentation#63438

Merged
fluttergithubbot merged 1 commit intoflutter:masterfrom
Hixie:restoration
Aug 21, 2020
Merged

More restoration documentation#63438
fluttergithubbot merged 1 commit intoflutter:masterfrom
Hixie:restoration

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Aug 10, 2020

No description provided.

@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Aug 10, 2020
@flutter-dashboard
Copy link

flutter-dashboard bot commented Aug 10, 2020

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.

@Hixie
Copy link
Contributor Author

Hixie commented Aug 10, 2020

cc @goderbauer

goderbauer
goderbauer approved these changes Aug 11, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

packages/flutter/lib/src/widgets/restoration_properties.dart Outdated
Comment on lines 86 to 87
Copy link
Member

@goderbauer goderbauer Aug 10, 2020

Choose a reason for hiding this comment

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

This is the default and could be omitted.

Copy link
Contributor Author

@Hixie Hixie Aug 11, 2020

Choose a reason for hiding this comment

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

removed

dev/snippets/config/templates/stateful_widget_restoration.tmpl Outdated
Copy link
Member

@goderbauer goderbauer Aug 10, 2020

Choose a reason for hiding this comment

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

Update https://github.com/flutter/flutter/blob/master/dev/snippets/config/templates/README.md to mention this new template?

packages/flutter/lib/src/widgets/restoration_properties.dart Outdated
Copy link
Member

@goderbauer goderbauer Aug 10, 2020

Choose a reason for hiding this comment

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

I believe OutlineButton is outdated, and should be replaced with a OutlinedButton (note the extra "d").

/cc @HansMuller

Copy link
Contributor

@HansMuller HansMuller Aug 11, 2020

Choose a reason for hiding this comment

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

Yes, please use OutlinedButton

Copy link
Contributor Author

@Hixie Hixie Aug 11, 2020

Choose a reason for hiding this comment

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

...

Copy link
Contributor Author

@Hixie Hixie Aug 11, 2020

Choose a reason for hiding this comment

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

i've filed #63510

goderbauer
goderbauer reviewed Aug 11, 2020
View reviewed changes
packages/flutter/lib/src/widgets/restoration_properties.dart Outdated
Copy link
Member

@goderbauer goderbauer Aug 11, 2020

Choose a reason for hiding this comment

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

For completeness, the RestorableInt needs to be disposed in dispose. See also #63497.

@Piinks Piinks added d: api docs Issues with https://api.flutter.dev/ documentation d: examples Sample code and demos labels Aug 11, 2020
goderbauer
goderbauer approved these changes Aug 12, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM, but cirrus is unhappy.

@goderbauer
Copy link
Member

goderbauer commented Aug 19, 2020

(PR triage fly-by comment): The analyzer's still unhappy.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Aug 20, 2020

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

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

@fluttergithubbot fluttergithubbot merged commit 39be8a4 into flutter:master Aug 21, 2020
smadey pushed a commit to smadey/flutter that referenced this pull request Aug 27, 2020
@Hixie @smadey
More restoration documentation (flutter#63438)
89d65cc
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@Hixie @mingwandroid
More restoration documentation (flutter#63438)
d55d419
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@goderbauer goderbauer goderbauer approved these changes

+1 more reviewer

@HansMuller HansMuller HansMuller left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Hixie @goderbauer @fluttergithubbot @HansMuller @googlebot @Piinks