[camera]refactor more types into separate files, and add unit tests #4704
Conversation
hellohuanlin
force-pushed
the
camera_refactor_other_enums
branch
from
e4b1a51 to
0d5d94f
Compare
| XCTAssertEqual(UIDeviceOrientationLandscapeRight, | ||
| FLTGetUIDeviceOrientationForString(@"landscapeLeft")); | ||
| XCTAssertEqual(UIDeviceOrientationLandscapeLeft, | ||
| FLTGetUIDeviceOrientationForString(@"landscapeRight")); |
There was a problem hiding this comment.
For some reason the left/right orientation is flipped, but this seems to be intentional for some undocumented reasons: https://github.com/flutter/plugins/pull/3390/files#r793213256
hellohuanlin
requested review from
cyanglaz,
jmagman and
stuartmorgan-g
and removed request for
bparrishMines
packages/camera/camera/example/ios/RunnerTests/DeviceOrientationTests.m
Outdated
Show resolved
Hide resolved
packages/camera/camera/example/ios/RunnerTests/ExposureModeTests.m
Outdated
Show resolved
Hide resolved
packages/camera/camera/example/ios/RunnerTests/FocusModeTests.m
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,19 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
I like the idea of splitting out things from CameraPlugin but this seems excessive, each header only has like 2 functions/typedef in them, and the function implementations are super simple. There's more boilerplate than logic in some of them. Same with the tests, I don't think we need 5 new test files each containing ~10 lines of asserts.
There's no fast rules here, it's preference, but lots of little interrelated files are hard to navigate, too.
What about a CameraProperties.h/m with everything you split out?
There was a problem hiding this comment.
I like the CameraProperties.h/m approach. Will update.
|
|
||
| #import "ExposureMode.h" | ||
|
|
||
| NSString *FLTGetStringForFLTExposureMode(FLTExposureMode mode) { |
There was a problem hiding this comment.
Nope, it's in the header file
hellohuanlin
force-pushed
the
camera_refactor_other_enums
branch
from
5e1399e to
39d5bae
Compare
hellohuanlin
added
the
waiting for tree to go green
3 participants
Breaking up CameraPlugin.m is part of optional cleanups discussed in the proposal. The monolithic CameraPlugin.m makes it hard to unit test other sub-tasks, so we want to do this clean up sooner.
Refactor the following types And added unit tests for them:
For some reason the left/right orientation is flipped, but this seems to be intentional for some unknown reason: https://github.com/flutter/plugins/pull/3390/files#r793213256
No version change: just moving code
Issue here: flutter/flutter#96429
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.