Skip to content

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented May 3, 2023

Publishes camera_android_camerax plugin as version 0.5.0.

Reminder to whoever lands this PR: Ping @/stuartmorgan to manually change package owner after landing this.

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.

@camsim99
Copy link
Contributor Author

camsim99 commented May 3, 2023

cc @gmackall for reference if you want to publish plugin before I get to it!

@camsim99
Copy link
Contributor Author

camsim99 commented May 8, 2023

@stuartmorgan for publishing this PR, I tried to excluding the plugin from building all_packages, because without excluding it, there's a conflict with camera_android: https://cirrus-ci.com/task/4968588499484672?logs=build_all_packages_debug#L49. Does this sound reasonable to you?

@stuartmorgan-g
Copy link
Collaborator

I tried to excluding the plugin from building all_packages, because without excluding it, there's a conflict with camera_android: https://cirrus-ci.com/task/4968588499484672?logs=build_all_packages_debug#L49. Does this sound reasonable to you?

What I would do is instead exclude camera_android, with an explanation:

# An application cannot depend directly on multiple federated implementations
# of the same plugin for the same platform, which means the app cannot 
# directly depend on both camera_android and camera_android_androidx.
# Since camera_android is endorsed, it will be included transitively
# already, so exclude it from the direct dependency list to allow including
# camera_android_androidx, to ensure they don't conflict at build time
# (since if they did, it would be impossible to use camera_android_androidx
# while camera_android is endorsed).
- camera_android

* Implements image streaming.
* Provides LifecycleOwner implementation for Activities that use the plugin that do not implement it themselves.
* Implements retrieval of camera information.
* Initial release of this `camera` implementation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest removing every entry except for this line (since from a client perspective there's nothing that these incremental changes are relative to, this being the first release), and then expanding this a bit to have a summary of what is implemented and a pointer to the readme for details.

version: 0.5.0

environment:
sdk: ">=2.18.0 <4.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Has this ever actually been run with 3.3? If not you may want to start it at 3.7.

@camsim99 camsim99 marked this pull request as ready for review May 9, 2023 16:52
@camsim99 camsim99 requested a review from stuartmorgan-g May 9, 2023 16:53
@camsim99
Copy link
Contributor Author

camsim99 commented May 9, 2023

@camsim99 camsim99 requested review from gmackall and reidbaker May 9, 2023 16:54
@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 (don't just cc him here, he won't see it! He's on Discord!).

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.

1 similar comment
@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 (don't just cc him here, he won't see it! He's on Discord!).

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.

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!

🚢

* Implements image streaming.
* Provides LifecycleOwner implementation for Activities that use the plugin that do not implement it themselves.
* Implements retrieval of camera information.
* Initial release of this `camera` implementation that supports image capture, video recording, displaying a live camera preview, and image streaming. See [`README.md`](README.md) for more details on the limitations of this implementation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: consider a formatted list:

* Initial release of this `camera` implementation that supports:
    * image capture
    * video recording
    * displaying a live camera preview
    * image streaming

  See [`README.md`](README.md) for more details on the limitations of this implementation.

@camsim99 camsim99 added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels May 9, 2023
@Hixie
Copy link
Contributor

Hixie commented May 9, 2023

test-exempt: configuration change

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label May 9, 2023
Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

LGTM!

@auto-submit auto-submit bot merged commit dfcb21c into flutter:main May 9, 2023
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
Publishes `camera_android_camerax` plugin as version 0.5.0.

**Reminder to whoever lands this PR**: Ping @/stuartmorgan to manually change package owner after landing this.
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 needs tests p: camera

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants