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.

Sign up for GitHub

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

Jump to bottom

Compatibility with swift playgrounds #2335

Merged
merged 2 commits into from
Sep 25, 2017
Merged

Compatibility with swift playgrounds #2335

merged 2 commits into from
Sep 25, 2017

Conversation

macteo
Copy link
Contributor

@macteo macteo commented Apr 6, 2017

  • Added missing import CoreGraphics where needed.
  • Added missing import UIKit where needed.

@macteo
Compatibility with swift playgrounds
5d4f8ac 
- Added missing import CoreGraphics where needed.
- Added missing import UIKit where needed.
@macteo
Added one last missing import CoreGraphics
409cebf
@pmairoldi
Copy link
Collaborator

Neat idea. Maybe you could add a playground the repo and we could use that instead of the demo project. Good work 👌

@liuxuan30
Copy link
Member

liuxuan30 commented Apr 10, 2017 •
edited

@petester42 we cool to merge some PRs like this? I think @danielgindi is too busy to review all the PRs :)

BTW, Seeing Travis new error type:

xcrun instruments -w '22FA2149-1241-469C-BF6D-462D3837DB72' || sleep 15
Instruments Usage Error: Unknown device specified: "22FA2149-1241-469C-BF6D-462D3837DB72"

While it has a bunch of devices

@macteo
Copy link
Contributor Author

macteo commented Apr 10, 2017 •
edited

I tried to solve the travis configuration error, unfortunately without success. Many PRs are failing due to the same issue.

@liuxuan30
Copy link
Member

@macteo did you just try to replace the device id with any one in the list, for example,

iPhone 7 (10.2) [34FA4749-C467-4D45-9F8E-E31AEDDC39C2] (Simulator)

@pmairoldi
Copy link
Collaborator

Ya that error is probably because some simulators got removed with Xcode 8.3. I'll look at this pull request soon and merge.

@thierryH91200 thierryH91200 mentioned this pull request Apr 15, 2017
Merged
@liuxuan30
Copy link
Member

@petester42 I update the device id in #2378

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@f23e872). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2335   +/-   ##
=========================================
  Coverage          ?   19.66%           
=========================================
  Files             ?      112           
  Lines             ?    13713           
  Branches          ?        0           
=========================================
  Hits              ?     2696           
  Misses            ?    11017           
  Partials          ?        0
Impacted Files Coverage Δ
...rts/Renderers/Scatter/ChevronUpShapeRenderer.swift 0% <ø> (ø)
Source/Charts/Highlight/Highlight.swift 0% <ø> (ø)
...Charts/Renderers/Scatter/CircleShapeRenderer.swift 0% <ø> (ø)
...urce/Charts/Renderers/Scatter/XShapeRenderer.swift 0% <ø> (ø)
Source/Charts/Filters/DataApproximator.swift 0% <ø> (ø)
...s/Renderers/Scatter/ChevronDownShapeRenderer.swift 0% <ø> (ø)
.../Charts/Renderers/Scatter/CrossShapeRenderer.swift 0% <ø> (ø)
...arts/Renderers/Scatter/TriangleShapeRenderer.swift 0% <ø> (ø)
...Charts/Renderers/Scatter/SquareShapeRenderer.swift 0% <ø> (ø)
Source/Charts/Components/Description.swift 100% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f23e872...409cebf. Read the comment docs.

liuxuan30
liuxuan30 reviewed Apr 24, 2017
View reviewed changes
Source/Charts/Data/Interfaces/IChartDataSet.swift
@@ -10,6 +10,7 @@
//

import Foundation
import CoreGraphics
Copy link
Member

@liuxuan30 liuxuan30 Apr 24, 2017 •
edited

Choose a reason for hiding this comment

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

It's interesting in my Xcode, it never warns or complains missing CoreGraphics, though IChartDataSet refers CGPoint

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since UIKit and AppKit imports CoreGraphics it is not needed.

Copy link
Contributor Author

@macteo macteo Apr 26, 2017 •
edited

Choose a reason for hiding this comment

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

Until you try to use it in a Playground

Copy link
Member

Choose a reason for hiding this comment

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

@macteo shall we consider it's playground bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so as it seems a strict requirement.

@pmairoldi pmairoldi self-assigned this Sep 15, 2017
@pmairoldi pmairoldi merged commit 2ccd171 into ChartsOrg:master Sep 25, 2017
PeterSrost pushed a commit to sokol8/Charts that referenced this pull request Oct 31, 2018
@pmairoldi
Merge pull request ChartsOrg#2335 from macteo/master
36941af 
Compatibility with swift playgrounds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmairoldi pmairoldi pmairoldi left review comments

@liuxuan30 liuxuan30 liuxuan30 left review comments

Assignees

@pmairoldi pmairoldi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@macteo @pmairoldi @liuxuan30 @codecov-io