Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framework/Test Targets for macOS #4

Closed
wants to merge 18 commits into from

Conversation

helje5
Copy link
Contributor

@helje5 helje5 commented Dec 12, 2017

I hope those do not confuse the Cocoapod.

This stuff makes it build as a Cocoa framework. Building things works fine, but the tests actually crash on macOS (same w/ spm test btw).

helje5 and others added 18 commits December 11, 2017 13:09
... sources in Sources, tests in Tests,
Package.swift, update README.md
... for SPM, do not use the CG* DefaultConstructors.
... more complete SPM instructions.
... to install section.
... all those static arrays, if only we had a Runtime ;-)
... tied it to Swift 4, doesn't work for 3, right?
- Linux has no `class_createInstance`, which is used for creating
  instances. This would need to be done differently here.
- Some issue w/ `NSObject` which is probably also due to the
  missing ObjC runtime
Add support for Swift Package Manager
... same files, different target for Cocoa.
... which crash, btw ;-)
@wickwirew
Copy link
Owner

What test fails? Is it the create class in the issue you made?

@helje5
Copy link
Contributor Author

helje5 commented Dec 12, 2017

Check issue #5. This is not related to Linux, it is just the same tests you run on iOS, fail on macOS. Seems to be an RC issue.

This PR is just adding a target to the Xcode project to build a macOS framework & tests. It should be fine to merge, unless they break the pods, don't know how that works.

@wickwirew
Copy link
Owner

wickwirew commented Dec 12, 2017

I think I may be reverting the SPM stuff so I'm going to hold off. Not a big fan of the massive restructure. When I first looked over the PR it didn't show all the the moved files and only showed about 400 line changes but once merged I saw the full extent of the changes. My mind was also clouded by excitement of my first contributor lol. I think all of the changes minus the file restructure are good and we need them in but I would like to keep the project structure the same. In swift 4 they did a full redesign of the SPM and I believe now you don't have to have all of the /Sources folder. Take a look at ReSwift and they support SPM but have a normal more traditional fire structure.

@helje5
Copy link
Contributor Author

helje5 commented Dec 13, 2017

Hey, no problem, it's your very own project! ;-) I'm not entirely sure where you see a "massive restructure", the files just got moved into Sources/Tests, thats it, but sure, if you don't like it, there is no obligation to accept PRs :-)

@wickwirew
Copy link
Owner

wickwirew commented Dec 13, 2017

I made a new develop and added the Package.swift and it builds fine for a test project as a dependency. So if you want to start fresh and branch off, so the git history isnt insane, that and copy some of the changes for linux/test over that would be good. I can do it if you really don't want too. Sorry again lol.

@helje5
Copy link
Contributor Author

helje5 commented Dec 13, 2017

BTW: This PR #4 is completely unrelated to SPM or Linux. It is about adding a regular macOS Target to the regular Xcode project you have. Do you want this? I think it would be good to get it working on macOS first, and then checkout Linux. I think it would be good to have at least a cannotCreateInstanceOnThisPlatform error.

(also: I'm personally fine w/ not having instance construction, I'm mostly interested in property write access, which does seem to work fine across platforms).

@helje5
Copy link
Contributor Author

helje5 commented Dec 13, 2017

Anyways, if you reverted the directory structure, this would need an update, so do not commit merge yet ;-)

@helje5
Copy link
Contributor Author

helje5 commented Dec 13, 2017

Actually, let me close this, I'm going to create a new PR.

@helje5 helje5 closed this Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants