Skip to content

[flutter_image] added example app#4353

Merged
auto-submit[bot] merged 36 commits intoflutter:mainfrom
Nitin-Poojary:flutter_image/adding_example_app
Oct 16, 2023
Merged

[flutter_image] added example app#4353
auto-submit[bot] merged 36 commits intoflutter:mainfrom
Nitin-Poojary:flutter_image/adding_example_app

Conversation

@Nitin-Poojary
Copy link
Contributor

@Nitin-Poojary Nitin-Poojary commented Jun 30, 2023 •
edited
Loading

Added example app for flutter_image package

closes #128699

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.

@Nitin-Poojary Nitin-Poojary force-pushed the flutter_image/adding_example_app branch from f15a546 to 598dd5f Compare July 5, 2023 11:02
@ditman
Copy link
Member

ditman commented Jul 6, 2023 •
edited
Loading

@Nitin-Poojary thanks for the contribution! However the branch on this PR has gotten a little bit messed up; it touches way more files than it should and includes commits not made by you (this is normally the sign of a bad rebase/update).

I'm going to mark this PR as a draft until the branch gets fixed, it cannot land in its current state.

@ditman ditman marked this pull request as draft July 6, 2023 19:30
@Nitin-Poojary Nitin-Poojary force-pushed the flutter_image/adding_example_app branch from 515d6e9 to 2c1987b Compare July 7, 2023 08:05
@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Jul 7, 2023 •
edited
Loading

Thank you for pointing out the issues. I apologize for any inconvenience caused. I have now fixed the problems. I believe it is now appropriate to mark this pull request as ready for review again.

@Nitin-Poojary Nitin-Poojary force-pushed the flutter_image/adding_example_app branch from 3ecc730 to 14e50f6 Compare October 10, 2023 15:21
@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Oct 11, 2023

Could you please look into the failing tests?

@stuartmorgan-g stuartmorgan-g self-requested a review October 16, 2023 17:04
stuartmorgan-g
stuartmorgan-g approved these changes Oct 16, 2023
View reviewed changes
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

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2023
@auto-submit auto-submit bot merged commit b96a6da into flutter:main Oct 16, 2023
@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Oct 16, 2023

Finally, I really appreciate how much you assisted me during this whole process. Thank you!

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Oct 16, 2023

Should I push more commits regarding TODO from this branch?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Oct 16, 2023

A follow-up would need to be a new PR in a new branch against the latest main.

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Oct 16, 2023

Okay.

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Oct 16, 2023

Since the example app hasn't appeared in the pinned version, should I wait for it to be included before proceeding?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Oct 16, 2023

I'm not sure what pinned version you are referring to; if you pull from main you will get the newest code, including the contents of this PR.

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 17, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2023
@engine-flutter-autoroll
b96a6da [flutter_image] added example app (flutter/packages#4353)
62002b1
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 17, 2023
@engine-flutter-autoroll
Roll Packages from 872d6d2 to d439062 (4 revisions) (#136724)
5739397 
flutter/packages@872d6d2...d439062

2023-10-17 ditman@gmail.com [gis_web] Adds FedCM toggle. (flutter/packages#5123)
2023-10-17 tarrinneal@gmail.com [quick_actions] Convert android to pigeon (flutter/packages#5099)
2023-10-16 70025277+Nitin-Poojary@users.noreply.github.com [flutter_image] added example app (flutter/packages#4353)
2023-10-16 engine-flutter-autoroll@skia.org Roll Flutter from b00216b to afc3916 (12 revisions) (flutter/packages#5156)

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 flutter-ecosystem@google.com,rmistry@google.com 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
@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Nov 3, 2023

String getUrlForAssetAsNetworkSource(String assetKey) {
return 'https://github.com/flutter/packages/blob/main/packages/flutter_image/example/$assetKey?raw=true';

This needs to use a pinned version, not main, so that it's not subject to unexpected breakage.

Could you please provide more details or clarification on this?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Nov 3, 2023

Sure, what information are you looking for?

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Nov 3, 2023

Regarding the use of the pinned version, as you suggested to use it instead of the main branch.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Nov 3, 2023

Sorry, I'm not sure what your question is. If it's whether this needs to continue to use a pinned version, then yes, that is still true per my previous comments.

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Nov 5, 2023

So using a pinned version to my asset image link will make it look like https://github.com/flutter/packages/blob/2e1673307ff7454aff40b47024eaed49a9e77e81/packages/flutter_image/example/$assetKey.png?raw=true right?, but this will return an error for every possible assetKey, since the example app does not exist at that hash.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Nov 5, 2023

but this will return an error for every possible assetKey, since the example app does not exist at that hash

A pinned version means you use a specific hash rather than something like main that changes over time. You would pin to a version that has the asset you want to reference.

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Nov 5, 2023

Okay, so that means I have to pull changes from main and create a separate hash and address that which will have my example app in it, am I correct? If so how to do it?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Nov 5, 2023

create a separate hash and address

I'm not sure what you mean. Every commit in a git repository has a hash. There are already dozens of hashes that have this example in them. The commit that landed this PR and the latest commit, for example, both have it.

@Nitin-Poojary
Copy link
Contributor Author

Nitin-Poojary commented Nov 5, 2023

Thank you for your response even on a Sunday. I was unsure about these hashes and how to obtain one, but your explanation that they are hashes of commits has cleared up my doubt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@tarrinneal tarrinneal tarrinneal approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: flutter_image platform-android platform-ios platform-linux platform-macos platform-web platform-windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[flutter_image] Add example app

4 participants

@Nitin-Poojary @ditman @stuartmorgan-g @tarrinneal

Comments