nerdishbynature / octokit.swift

A Swift API Client for GitHub and GitHub Enterprise
MIT License
488 stars 126 forks source link

Fix swift build #102

Closed bofeizhu closed 4 years ago

bofeizhu commented 4 years ago

Fixed #101

bofeizhu commented 4 years ago

I don't know why Travis kept running on the Xcode9 image. Fixed .travis.yml as well.

pietbrauer commented 4 years ago

Thanks for the effort but I have a hard time to process what the actual problem was.

  1. You moved all files to Sources and removed the custom path from the Swift package -> Why?
  2. Readded all files to the Xcodeproj -> How is this related to SwiftPM?
  3. Introduced redundancy in the .travis.yml

Please help me understand so we can get this fixed for Danger.

bofeizhu commented 4 years ago

Hi Piet, thank you for the reply! Here are the answers to your questions.

  1. I don’t know if it’s a bug introduced in Swift 5.2 (Xcode 11.4) or if it’s a change of design. The new SPM includes the .h and .plist files if we refer the source files by path and root and gives us OctoKit contains mixed language source files error when we swift build. That’s why I moved source files to the SPM’s default directory (Sources/PackageName) and updated the Package.swift file. This update fixed #101.

  2. Because of 1, I have to update the xcodeproj to refer to new file locations.

  3. You can refer to my comment on top. Travis failed to initiate the correct Xcode image last night for my PR. You can refer to the Travis logs of my early commits in this PR. When default language was set to generic, it initiated Xcode 9 image for all the macOS workers. Updated the language to swift solved this issue. I’m not an expert of Travis so I’m not sure if this is a bug/permanent change on their end.

bofeizhu commented 4 years ago

@pietbrauer bump^ just in case you missed my comment 😉.

MaxDesiatov commented 4 years ago

Hope this can be merged soon, as it blocks GitBuddy from building with Xcode 11.4

Killectro commented 4 years ago

It also blocks Danger Homebrew Tap as well.

f-meloni commented 4 years ago

@bofeizhu I think you should be able to restore travis, I saw some issues with it before, but it looks working again

bofeizhu commented 4 years ago

@f-meloni Just tried, not working... Have to set language to Swift https://travis-ci.org/github/nerdishbynature/octokit.swift/builds/668130190?utm_source=github_status&utm_medium=notification As you can see here, the Xcode 9 image is used when I reverse the language to generic. I think this is a relatively new bug in Travis.

bofeizhu commented 4 years ago

@pietbrauer I updated the .travis.yml again. I believe this is the version with the least redundancy that can workaround Travis's bug at the moment. cc @f-meloni

pietbrauer commented 4 years ago

I've been working on a less invasive approach (#103) where all files stay in place and the only thing that changes is the one causing the problem, Package.swift.

@bofeizhu Could you please have a look if this fixes your problem? I am also happy to merge your PR once it restores the original structure.

bofeizhu commented 4 years ago

@pietbrauer LGTM! I'm closing this PR. Please merge #103 instead. Thx!