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

Enable swift package manager #63

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

Make1978
Copy link

@Make1978 Make1978 commented Nov 15, 2021

Add package.swift
Move sources to Sources folder
Move test to Test folder

Add GitHub action: Build and Test a Swift package

run tests
swift test

@@ -0,0 +1,19 @@
name: Swift
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding a test for the build is fantastic, thank you.

.gitignore Outdated
.build/
xcuserdata
DerivedData/
*.xcodeproj
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be what's breaking the project. If I'm not mistaken, the standard Xcode project is going to get nuked by this.

I'd like peertalk when simply checking out the code, opening Xcode and hitting "Build and Run".

Let's work out a way so that, and Swift PM can live side by side (if possible, if not, let's discuss).

@@ -10,7 +10,7 @@ Pod::Spec.new do |spec|
spec.summary = 'iOS and OS X Cocoa library for communicating over USB and TCP.'

spec.source = { :git => "https://github.com/rsms/PeerTalk.git", :tag => '0.1.0' }
spec.source_files = 'peertalk/*.{h,m}'
spec.source_files = 'Sources/peertalk/*.{h,m}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy for this change of location, seems like an improvement to the project structure.

@@ -1,5 +1,5 @@
#import "PTChannel.h"
#import "PTPrivate.h"
#import "include/PTChannel.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a necessary change to make things work with Swift PM? If no then, can we keep the scope of this change to adding Swift PM and not moving these headers out too?

#import <peertalk/PTProtocol.h>
#import <peertalk/PTUSBHub.h>
#import <peertalk/PTDefines.h>
#import "PTProtocol.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem right to me. Xcode already allows you to Always Search User Header Paths (I forget if that's the exact setting name), and it can find these headers fine with the original <> form.

Copy link
Collaborator

@jonathandann jonathandann left a comment

Choose a reason for hiding this comment

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

Thank you for contributing! I really appreciate you adding swift PM support.

Something seems to be broken with the Xcode project when I check this out. Xcode can't open peertalk.xcodeproj.

Xcode gives me the following error:

Project /Users/jonathan/source/peertalk/peertalk.xcodeproj cannot be opened because it is missing its project.pbxproj file.

@cbjeukendrup
Copy link

Xcode gives me the following error:

Project /Users/jonathan/source/peertalk/peertalk.xcodeproj cannot be opened because it is missing its project.pbxproj file.

That's because the PR effectively deletes the Xcode project, but you will probably still see the .xcodeproj file when you checkout this PR, because an .xcodeproj file is actually a directory, and Git never deletes (empty) directories.

I think the Xcode project is still needed, also for the examples.

PR #56 seems to have a different approach, keeping the Xcode project.

@Make1978
Copy link
Author

added peertalk.xcodeproj/

Example projects are ignored. I Will check those later this week :)

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.

3 participants