jcavar / refresher

DEPRECATED: Pull to refresh in Swift
MIT License
873 stars 99 forks source link

Xcode8 GM + swift3 migration #34

Closed appsailor closed 8 years ago

jcavar commented 8 years ago

Very nice. Thanks! I will try to review this as soon as possible.

appsailor commented 8 years ago

That was part of the migration tool initiated changes. So kept them as suggested. After all, these are just demo classes right?

On Wed, Sep 21, 2016 at 9:37 PM, Josip Ćavar notifications@github.com wrote:

@jcavar commented on this pull request.

In PullToRefreshDemo/BeatAnimator.swift https://github.com/jcavar/refresher/pull/34#pullrequestreview-915428:

@@ -28,52 +28,52 @@ import QuartzCore

class BeatAnimator: UIView, PullToRefreshViewDelegate {

  • private let layerLoader = CAShapeLayer()
  • private let layerSeparator = CAShapeLayer()
  • fileprivate let layerLoader = CAShapeLayer()
  • fileprivate let layerSeparator = CAShapeLayer()

Same here, why fileprivate?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jcavar/refresher/pull/34#pullrequestreview-915428, or mute the thread https://github.com/notifications/unsubscribe-auth/AF03CpXMMPlHgnN7GbiolOsyDD0O6ctCks5qsPrXgaJpZM4KCRQp .

appsailor commented 8 years ago

In case we are supporting other platforms in the future, the suggested approach is to use deployment_target.

see https://guides.cocoapods.org/syntax/podspec.html#group_multi_platform_support

As opposed to the platform attribute, the deployment_target attribute allows to specify multiple platforms on which this pod is supported — specifying a different deployment target for each.

On Wed, Sep 21, 2016 at 10:48 PM, Josip Ćavar notifications@github.com wrote:

@jcavar commented on this pull request.

In Refresher.podspec https://github.com/jcavar/refresher/pull/34#pullrequestreview-925988:

  • s.platform = :ios, "8.0"
  • s.ios.deployment_target = '9.0'

Why was this needed?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jcavar/refresher/pull/34#pullrequestreview-925988, or mute the thread https://github.com/notifications/unsubscribe-auth/AF03Cj6-Bo5toTy5YL0sSA2o72iXmr77ks5qsQuagaJpZM4KCRQp .