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

Charts changes for Swift 2.3 #1145

Closed
wants to merge 4 commits into from
Closed

Charts changes for Swift 2.3 #1145

wants to merge 4 commits into from

Conversation

opswhisperer
Copy link

@opswhisperer opswhisperer commented Jun 15, 2016

If using cocoapods and Xcode 8 add the following to ensure the framework is set for Swift 2.3. Optionally modify this to only change the Charts pod

post_install do |installer|
  installer.pods_project.targets.each do |target|
    target.build_configurations.each do |config|
      config.build_settings['SWIFT_VERSION'] = '2.3'
    end
  end
End

If its not a swift pod it still gets added but ignored, if it its a Swift pod then it sets the Use Legacy Swift Language Version build setting to Yes

@liuxuan30
Copy link
Member

@danielgindi shall we consider 2.3 or we jump to 3.0 directly? I did see more work for 3.0 while converting. 2.3 is much easier to convert automatically. 2.3 is the version to get started with Xcode 8 very quickly

@opswhisperer
Copy link
Author

I’d love to go to 3 directly but I have a lot more dependancies that need to move too.
Theoretically I think you could have a Swift 3 framework in a 2.3 project since they each have their own build settings.

@jerryhjones
Copy link

There are a lot of project related changes in there that you problem shouldn't include in the pull request. Thinks like...

DevelopmentTeam = 4ZBD65B36X;
DevelopmentTeamName = "Robert Bell";
LastSwiftMigration = 0800;

@opswhisperer
Copy link
Author

True, I was being lazy and didn't want to break my project. He's going to have to replace those anyway as its a part of the new code signing system. Normally I'd .gitignore the project file but thats where you have to set the swift version.

@liuxuan30
Copy link
Member

I compare this branch with my swift 2.3 branch, and actually you are having some info like development team, xcscheme that I don't have in diff. I think it's your .gitignore file issue? Because the one from the library seems filted xcscheme.

@opswhisperer
Copy link
Author

I would take your 2.3 branch one mine, the Team info in the diff was me trying to get Cocoapods to respect those new settings in Xcode 8 so I pushed them to my Charts repo.

On Jun 16, 2016, at 8:30 AM, Xuan notifications@github.com wrote:

I compare this branch with my swift 2.3 branch, and actually you are having some info like development team, xcscheme that I don't have in diff. I think it's your .gitignore file issue? Because the one from the library seems filted xcscheme.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #1145 (comment), or mute the thread https://github.com/notifications/unsubscribe/AEYER_u9KOEPDA1Qpg-1q1HvJ0F0raH8ks5qMWwDgaJpZM4I2iRS.

@liuxuan30
Copy link
Member

I see, I don't use pod for my repo, just the same clone of the upstream branch.

@liuxuan30
Copy link
Member

@opswhisperer would you fix the irrelevant diffs or I create a new 2.3 merge?

@opswhisperer
Copy link
Author

I can fix it, might be later today.

On Jun 16, 2016, at 9:00 AM, Xuan notifications@github.com wrote:

@opswhisperer https://github.com/opswhisperer would you fix the irrelevant diffs or I create a new 2.3 merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #1145 (comment), or mute the thread https://github.com/notifications/unsubscribe/AEYER7usQOqRcjrYt6iFYmG222aK2HVHks5qMXMcgaJpZM4I2iRS.

@opswhisperer
Copy link
Author

Updated, left only the swift changes and the build settings recommended by Xcode

@liuxuan30
Copy link
Member

I am closing this and use #1163 instead.

@liuxuan30 liuxuan30 closed this Jun 21, 2016
@liuxuan30
Copy link
Member

liuxuan30 commented Jun 21, 2016

oops, my bad, I just saw you updated your PR.

However, an interesting thing is for ChartPlatform.swift, you and I have major different results.. Do you use Xcode 8 converter to do it? Or you manually did that?

@liuxuan30 liuxuan30 reopened this Jun 21, 2016
@liuxuan30
Copy link
Member

liuxuan30 commented Jun 21, 2016

BTW, please choose Swift-2.3 branch for this PR. I have created this branch and merged a working version for swift 2.3.

I am also seething some minor differences, like whole module optimization vs single file optimization.

@opswhisperer you could add those you think are better or more reasonable. Thank you!

@opswhisperer
Copy link
Author

All of those changes are from the migration tool. I didn’t do any manually

@liuxuan30
Copy link
Member

liuxuan30 commented Jul 29, 2016

closing; we had a swift 2.3 branch for this. I am not sure how long will swift 2.3 will exists, but currently 2.3 and 3.0 branch is for testing iOS 10, so once it is released, I think convert all in once at that time based on master is a better choice than continuous merging

@liuxuan30 liuxuan30 closed this Jul 29, 2016
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