Skip to content
This repository was archived by the owner on May 3, 2021. It is now read-only.

watchOS#72

Merged
kean merged 1 commit intokean:masterfrom
bassrock:watchOS
Sep 9, 2015
Merged

watchOS#72
kean merged 1 commit intokean:masterfrom
bassrock:watchOS

Conversation

@bassrock
Copy link
Copy Markdown
Contributor

@bassrock bassrock commented Sep 5, 2015

This seems to work and fix #15, The same approach can also be used to add watchUI classes and OS X UI Classes.

@bassrock bassrock changed the title Watch os watchOS Sep 5, 2015
@kean
Copy link
Copy Markdown
Owner

kean commented Sep 5, 2015

Hi @bassrock, thanks for the pull request! I'll take a look into it as soon as I can.

From what I saw at the first glance:

  1. Please branch of the develop branch instead of master
  2. Instead of creating a different subspec with UI components for each platform I would rather keep 'UI' subspec and specify source files per platform:
    s.subspec "UI" do |ss|
        ss.dependency "DFImageManager/Core"
        ss.source_files = "DFImageManager/Source/UI/**/*.{h,m}" # shared files
        ss.ios.source_files = "DFImageManager/Source/UI/iOS/*.{h,m}" # platform specific files
    end

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You can't use mach_absolute_time() like that. It returns time in processing units, not seconds like CACurrentMediaTime(). Conversion between mach_absolute_time() units and seconds is quite complicated.

Anyway, both functions are useful when you need to measure time in terms of nanoseconds, which is not the case here. NSDate looks like a good candidate, but it's rather heavy and this code needs to run on the main thread. I would rather use something that is faster.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea the reasoning here is that CACurrentMediaTime is not available on the watch. Any suggestions for a replacement works.

@bassrock
Copy link
Copy Markdown
Contributor Author

bassrock commented Sep 5, 2015

Ok will update the specs in a bit. Also this is branched off of develop.

@bassrock
Copy link
Copy Markdown
Contributor Author

bassrock commented Sep 5, 2015

I think CFAbsoluteTimeGetCurrent works.

@kean
Copy link
Copy Markdown
Owner

kean commented Sep 6, 2015

CFAbsoluteTimeGetCurrent is ok 👍

Subspec name is still iOSUI s.subspec "iOSUI" do |ss|. Could you please rename it back to "UI" and I think it would be ok to merge this?

@kean kean mentioned this pull request Sep 9, 2015
@kean
Copy link
Copy Markdown
Owner

kean commented Sep 9, 2015

Merging, thanks for your contribution @bassrock 👍
P.S. If you want to submit more, please branch of the develop branch and create pull requests for the develop branch, thanks :)

kean added a commit that referenced this pull request Sep 9, 2015
@kean kean merged commit c5be8c2 into kean:master Sep 9, 2015
@kean
Copy link
Copy Markdown
Owner

kean commented Sep 12, 2015

I've removed entire xcodeproj with framework targets and moved to classic CocoaPod project structure with a single Xcode project. Now there is no need to manage two build configuration at the same time, we rely on a single build configuration, which is podpsec and CocoaPods.

kean added a commit that referenced this pull request Sep 15, 2015
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.

watchOS support

2 participants