Skip to content

Conversation

foxtrotravi
Copy link
Contributor

@foxtrotravi foxtrotravi commented Nov 3, 2023

Description of PR:

Adding an example app to showcase the usage of cupertino_icons. This addition will also increase the pub points of the package and will be helpful for other developers to quickly find relevant examples.

Fixes flutter/flutter#137682

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 [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • 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.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

This adds a large amount of boilerplate, and doesn't appear to offer any benefits that https://api.flutter.dev/flutter/cupertino/CupertinoIcons-class.html doesn't. An example/example.md file that links to https://api.flutter.dev/flutter/cupertino/CupertinoIcons-class.html seems like it would achieve the goals of the linked issue in a much simpler way.

@foxtrotravi
Copy link
Contributor Author

Okay @stuartmorgan I will update this PR to remove the app & add an example.md file which links to the documentation.

@foxtrotravi foxtrotravi force-pushed the add_example_to_cupertino_icons branch from 86a6c9f to fb8daf3 Compare November 5, 2023 06:33
@foxtrotravi
Copy link
Contributor Author

@stuartmorgan I have updated the PR. Please review.

@foxtrotravi foxtrotravi force-pushed the add_example_to_cupertino_icons branch 3 times, most recently from 46cedc2 to a365fd4 Compare November 24, 2023 16:52
@foxtrotravi foxtrotravi force-pushed the add_example_to_cupertino_icons branch 2 times, most recently from 787b811 to 3a76ff3 Compare December 2, 2023 05:18
@foxtrotravi foxtrotravi force-pushed the add_example_to_cupertino_icons branch 2 times, most recently from 0a2746d to d779fa6 Compare December 14, 2023 09:15
@stuartmorgan-g
Copy link
Collaborator

Once #5775 is reviewed and landed, merge main into this PR should fix the CI failures.

@foxtrotravi
Copy link
Contributor Author

Once #5775 is reviewed and landed, merge main into this PR should fix the CI failures.

Okay @stuartmorgan

@stuartmorgan-g
Copy link
Collaborator

The failure this time is something that needs to be fixed in the PR due to a recently enabled lint; see https://github.com/flutter/packages/pull/5717/files#diff-86bbf9121116e918f9f79ccbb484b1d77358f1a30f092129f30333542f614c75 for an example of a fix for the same lint.

@foxtrotravi foxtrotravi force-pushed the add_example_to_cupertino_icons branch from 6a2025b to 3f24c60 Compare January 28, 2024 09:42
@foxtrotravi
Copy link
Contributor Author

@stuartmorgan please review

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with nit. To @MitchellGoodwin for secondary review.

Copy link

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM!

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 12, 2024
@auto-submit auto-submit bot merged commit d1f1f0f into flutter:main Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 13, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 13, 2024
flutter/packages@0a69259...9385bbb

2024-02-13 [email protected] Convert startProductRequest(), finishTransaction(), restoreTransactions(), presentCodeRedemptionSheet() to pigeon (flutter/packages#6032)
2024-02-13 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20231013 to 20240205 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter/packages#6096)
2024-02-12 [email protected] [local_auth] Rename iOS classes (flutter/packages#6108)
2024-02-12 [email protected] [video_player_android] Handle BehindLiveWindowException (flutter/packages#5869)
2024-02-12 [email protected] [in_app_purchase] Add alternative billing apis for android (flutter/packages#6056)
2024-02-12 [email protected] [webview_flutter] Update compileSdk to 34 (flutter/packages#6106)
2024-02-12 [email protected] [cupertino_icons] Add example to cupertino icons (flutter/packages#5312)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: cupertino_icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cupertino_icons] Add example
3 participants