Skip to content

Conversation

@PROGrand
Copy link
Contributor

@PROGrand PROGrand commented Oct 24, 2023

Platform implementations of federated plugin

This is the platform implementations part of camera PR #3586.

camera_platform_interface: 2.6.0 merged and published in PR #3615.

Now repeating steps 3,4 (see Changing federated plugins), because camera/camera depends on implementations camera/camera_android, camera/camera_web etc.

@PROGrand PROGrand marked this pull request as draft March 6, 2024 23:03
@PROGrand PROGrand marked this pull request as ready for review March 7, 2024 11:52
@mossmana mossmana changed the title Camera with MediaSettings: platform implementations (federated) [camera] Camera with MediaSettings: platform implementations (federated) Mar 13, 2024
@jmagman
Copy link
Member

jmagman commented Mar 13, 2024

@stuartmorgan this is blocked on your review

@PROGrand PROGrand requested a review from stuartmorgan-g March 14, 2024 19:26
@PROGrand
Copy link
Contributor Author

PROGrand commented Mar 14, 2024

@stuartmorgan Thank, you for review. Your suggestions are applied.

@vashworth
Copy link
Contributor

@stuartmorgan Can you take a look when you have time?

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.

Sorry for the delay again; just a few last small things.

@PROGrand PROGrand requested a review from stuartmorgan-g April 5, 2024 11:46
@PROGrand
Copy link
Contributor Author

PROGrand commented Apr 5, 2024

@stuartmorgan suggestions applied

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! Could you rebase and squash a few of the commits together to get this under 250 commits, so that the CLA check will pass without intervention?

suggestions applied

AssertPositiveNumberOrNil: macro to inline

formatted

.class to +class

Update packages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m

Co-authored-by: Jenn Magder <[email protected]>

applied suggestions. 02/25/2024.

dependency injection (DI) variant for unit test

ObjC suggestions applied

suggestion applied

updated camera_platform_interface versions

reverted changes to camera

merged 01/07/2024

refactored and co0mmented warning suppressions. renamed MediaRecorderBuilder.RecordingParameters

merged 12/07/2023
@PROGrand PROGrand force-pushed the camera-with-settings-platform-implementations branch from faa7b8f to adf6929 Compare April 5, 2024 14:14
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.

A bunch of files showed as changed since review, and because of the squash I can't tell if that's a GitHub artifact of the squash, or actual changes, so I re-reviewed those files. That turned up a few more minor comments which may be things I just didn't previously review, but I've flagged them regardless. Sorry that means one more quick pass.

@PROGrand PROGrand requested a review from stuartmorgan-g April 5, 2024 15:27
@PROGrand
Copy link
Contributor Author

PROGrand commented Apr 5, 2024

@stuartmorgan resolved

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.

Thanks!

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 5, 2024
@auto-submit auto-submit bot merged commit caf48bc into flutter:main Apr 5, 2024
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 federated: partial_changes PR that contains changes for only a single package of a federated plugin change p: camera platform-android platform-ios platform-macos platform-web platform-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants