Skip to content
This repository was archived by the owner on Jun 15, 2019. It is now read-only.

Conversation

@paynerc
Copy link
Contributor

@paynerc paynerc commented Dec 12, 2018

No description provided.

Copy link
Contributor

@ceaglest ceaglest left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Just minor feedback.

Since this is the first PR into the feature branch we should also cross-link back to master in README.md, and link directly to the preview docs URLs.

self.previewView.mirror = (device.position == AVCaptureDevicePositionFront);
}
}];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What should we do if the front camera is not available? I think there might have been one iPod Touch that we don't actually support like this...

Copy link
Contributor

@ceaglest ceaglest Dec 13, 2018

Choose a reason for hiding this comment

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

Maybe we should assert or otherwise end the program? Or print an error, or show a UIAlertView? It seems like a case we were not expecting. See also flipCamera below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What ever we decide to do, we also need to do in the Swift code as well. It has the same behavior as this.

Podfile Outdated

abstract_target 'TwilioVideo' do
pod 'TwilioVideo', '~> 2.5'
pod 'TwilioVideo', '~> 2.6.0-preview1'
Copy link
Contributor

Choose a reason for hiding this comment

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

2.6.0-preview2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dang it.. Copy/Paste error. lol.

I had to start with preview1 so I could at least have it have most of the header file links that CocoaPods does, and then copied the RC in manually. Nice catch.

Copy link
Contributor

@piyushtank piyushtank left a comment

Choose a reason for hiding this comment

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

I agree with @ceaglest 's comments. Lets address them and keep the PR ready to merge fore preview2 release.

Copy link
Contributor

@ceaglest ceaglest left a comment

Choose a reason for hiding this comment

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

Looks good, my only remaining feedback is about handling devices without a front camera (an edge case, to be sure).


* [Getting Started](https://www.twilio.com/docs/video/ios-v2-getting-started)
* [Docs](https://twilio.github.io/twilio-video-ios/docs/latest/index.html)
* [Docs](https://twilio.github.io/twilio-video-ios/docs/2.6.0-preview2/index.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

self.previewView.mirror = (device.position == AVCaptureDevicePositionFront);
}
}];
}
Copy link
Contributor

@ceaglest ceaglest Dec 13, 2018

Choose a reason for hiding this comment

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

Maybe we should assert or otherwise end the program? Or print an error, or show a UIAlertView? It seems like a case we were not expecting. See also flipCamera below.

@paynerc paynerc merged commit 1664ea5 into 2.6.0-preview Dec 14, 2018
@paynerc paynerc deleted the task/ISDK-2303 branch December 14, 2018 03:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants