Skip to content

Const constructor audit. #76162

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 18, 2021
Merged

Const constructor audit. #76162

merged 1 commit into from
Feb 18, 2021

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Feb 16, 2021 •
edited
Loading

Where possible this uses this boilerplate text instead of bespoke text
that doesn't say anything more useful:

  /// Abstract const constructor. This constructor enables subclasses to provide
  /// const constructors so that they can be used in const expressions.

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 listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Feb 16, 2021
@flutter-dashboard
Copy link

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.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@google-cla google-cla bot added the cla: yes label Feb 16, 2021
@Hixie
Copy link
Contributor Author

Hixie commented Feb 17, 2021

cc @HansMuller

HansMuller
HansMuller approved these changes Feb 17, 2021
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller 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/actions.dart Outdated
@@ -1561,7 +1562,7 @@ class DoNothingAction extends Action<Intent> {
/// * [WidgetsApp.shortcuts], which defines the shortcuts to use in an
/// application (and defaults to [WidgetsApp.defaultShortcuts]).
class ActivateIntent extends Intent {
/// Creates a const [ActivateIntent] so subclasses can be const.
/// Creates ain intent that activates the currently focused control.
Copy link
Contributor

Choose a reason for hiding this comment

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

ain => an

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, fixed

packages/flutter/lib/src/widgets/preferred_size.dart
@@ -111,7 +111,7 @@ abstract class PreferredSizeWidget implements Widget {
/// its preferred size.
/// * [AppBar] and [TabBar], which implement PreferredSizeWidget.
class PreferredSize extends StatelessWidget implements PreferredSizeWidget {
/// Creates a widget that has a preferred size.
/// Creates a widget that has a preferred size that the parent can query.
Copy link
Contributor

Choose a reason for hiding this comment

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

the parent => any ancestor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would someone other than a parent query it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've added some more docs here but i'm pretty sure "parent" is actually right here.

Copy link
Contributor

Choose a reason for hiding this comment

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

My mistake, parent is fine.

@Hixie
Copy link
Contributor Author

Hixie commented Feb 17, 2021

Landing on green, please let me know if I'm wrong about the parent/ancestor thing and I'll fix in a subsequent PR. Thanks!

@Hixie
Const constructor audit.
8a05206 
Where possible this uses this boilerplate text instead of bespoke text
that doesn't say anything more useful:

```
  /// Abstract const constructor. This constructor enables subclasses to provide
  /// const constructors so that they can be used in const expressions.
```
@fluttergithubbot
Copy link
Contributor

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

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

@fluttergithubbot fluttergithubbot merged commit 3419842 into flutter:master Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@HansMuller HansMuller HansMuller approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. f: material design flutter/packages/flutter/material repository. 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.
3 participants
@Hixie @fluttergithubbot @HansMuller