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

Removed methods and properties deprecated in 1.0 #2996

Merged
merged 5 commits into from
Dec 18, 2017
Merged

Removed methods and properties deprecated in 1.0 #2996

merged 5 commits into from
Dec 18, 2017

Conversation

jjatie
Copy link
Collaborator

@jjatie jjatie commented Nov 13, 2017

These produce quite a bit of clutter.
It has been 2-3 years since their deprecation, it seems reasonable to remove them now.

If this is not to be accepted now, when is a good time to resubmit this PR?

@codecov-io
Copy link

codecov-io commented Nov 14, 2017

Codecov Report

Merging #2996 into master will increase coverage by 0.53%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2996      +/-   ##
==========================================
+ Coverage   19.45%   19.99%   +0.53%     
==========================================
  Files         113      113              
  Lines       16044    15617     -427     
  Branches      247      247              
==========================================
  Hits         3122     3122              
+ Misses      12884    12457     -427     
  Partials       38       38
Impacted Files Coverage Δ
...a/Implementations/Standard/BarChartDataEntry.swift 2.56% <ø> (+0.09%) ⬆️
Source/Charts/Charts/PieChartView.swift 0% <ø> (ø) ⬆️
...a/Implementations/Standard/PieChartDataEntry.swift 0% <ø> (ø) ⬆️
Source/Charts/Utils/ChartUtils.swift 45.98% <ø> (+7.83%) ⬆️
...ta/Implementations/Standard/LineChartDataSet.swift 29.85% <ø> (+5.15%) ⬆️
Source/Charts/Components/Legend.swift 55.02% <ø> (+30.94%) ⬆️
Source/Charts/Components/AxisBase.swift 46.57% <ø> (+1.24%) ⬆️
Source/Charts/Charts/ChartViewBase.swift 22.14% <0%> (+0.78%) ⬆️

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 159e0f7...a33f952. Read the comment docs.

@liuxuan30
Copy link
Member

liuxuan30 commented Nov 22, 2017

have you checked what's the last date when adding those deprecated tag, I remember it was when Chart 3.0 released? I'm ok to remove them.
@petester42 what you think

BTW, it has conflicts :)

@jjatie
Copy link
Collaborator Author

jjatie commented Nov 22, 2017

I looked for a bit, but gave up looking for the merge. They are marked as deprecated in 1.0, so it doesn't make sense to me that they would be introduced in 3.0.

@pmairoldi
Copy link
Collaborator

It was a misunderstanding on what the number there meant. It only applies to OS versions. It doesn’t read our frameworks version. They could have been deprecated in 2.0 or 3.0 I don’t know when though.

@liuxuan30
Copy link
Member

but we can remove them now, right?

@jjatie
Copy link
Collaborator Author

jjatie commented Nov 25, 2017

Searching through the commits I've found that these methods were all deprecated between Apr 8, 2016 and Sept 21, 2016. If we aren't removing them right now, can we mark this for 4.0.0


/// MARK: - Bridging functions

internal class func bridgedObjCGetNSUIColorArray (swift array: [NSUIColor?]) -> [NSObject]
Copy link
Member

@liuxuan30 liuxuan30 Nov 27, 2017

Choose a reason for hiding this comment

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

why is this included?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These only existed to support the deprecated functions. Since I am removing the deprecated functions in this PR, these methods are no longer needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, this could have been written as let myObjArray = myColorArray as! [NSObject]

@liuxuan30
Copy link
Member

liuxuan30 commented Nov 27, 2017

I'm supporting, like after we finish refactoring and some other enhancements and make a major release, since many things changed and seems it's the right time to remove those APIs

@liuxuan30
Copy link
Member

liuxuan30 commented Dec 5, 2017

@jjatie will your future commits involve these deprecated methods? If so, I favor to merge soon.

@jjatie
Copy link
Collaborator Author

jjatie commented Dec 5, 2017

My other PRs don't touch these deprecated methods at all. If we are not merging soon, then this should be in 4.0.0. Given the deprecations are between 14 and 20 months old, I think it's acceptable to do this before 4.0.0

@liuxuan30
Copy link
Member

OK, I will hold this for the moment, we shall have a release after the refinement, so just a matter of time and order.

@liuxuan30 liuxuan30 added this to 4.0 change in More Swift Dec 12, 2017
@liuxuan30
Copy link
Member

liuxuan30 commented Dec 18, 2017

shall we merge this now or for 4.0, as #3034 has changing some.

@liuxuan30 liuxuan30 mentioned this pull request Dec 18, 2017
@liuxuan30
Copy link
Member

merging this, to make things easier.

@liuxuan30 liuxuan30 merged commit ecdd295 into ChartsOrg:master Dec 18, 2017
@jjatie jjatie deleted the depracated-1.0-removal branch December 18, 2017 12:05
@liuxuan30 liuxuan30 moved this from PRs that's target for 4.0 to PRs target for Master in More Swift Dec 19, 2017
FreddyZeng added a commit to FreddyZeng/Charts that referenced this pull request Jan 2, 2018
* 'master' of https://github.com/danielgindi/Charts: (23 commits)
  Update ViewPortHandler.swift (ChartsOrg#3143)
  add option to build demo projects unit tests on iOS (ChartsOrg#3121)
  Replaced relevant `ChartUtils` methods with `Double` extensions (ChartsOrg#2994)
  Update 4.0.0 with master (ChartsOrg#3135)
  Removed redundant ivars in BarLineChartViewBase (ChartsOrg#3043)
  fix ChartsOrg#1830. credit from ChartsOrg#2049 (ChartsOrg#2874)
  Makes ChartsDemo compiling again (ChartsOrg#3117)
  Fixed using wrong axis (Issue ChartsOrg#2257)
  Removed methods and properties deprecated in 1.0 (ChartsOrg#2996)
  for ChartsOrg#3061 revert animationUpdate() and animationEnd() not trigger crash if subclass does nothing
  The backing var is not necessary. (ChartsOrg#3000)
  Replaced `ChartUtils.Math` in favour of an extension on `FloatingPoint` (ChartsOrg#2993)
  Minor logic cleanup (ChartsOrg#3041)
  Fix a bug may cause infinite loop. (ChartsOrg#3073)
  for ChartsOrg#2745. chart should be weak.
  fileprivate -> private (ChartsOrg#3042)
  Removed `isKind(of:)`
  Removed @objc from internal properties
  Fixes for PR
  Made use of `==` where appropriate to simplify logic
  ...

# Conflicts:
#	Source/Charts/Data/Implementations/Standard/LineChartDataSet.swift
#	Source/Charts/Renderers/BarChartRenderer.swift
@liuxuan30 liuxuan30 moved this from PRs target for Master to Done in More Swift Jan 7, 2018
FreddyZeng added a commit to FreddyZeng/Charts that referenced this pull request Jan 20, 2018
* 'master' of https://github.com/danielgindi/Charts: (34 commits)
  Fixed X-Axis Labels Not Showing (ChartsOrg#3154) (ChartsOrg#3174)
  fix programatical unhighlighting for BarCharView (ChartsOrg#3159)
  Give the users customizable axis label limits (Fixes ChartsOrg#2085) (ChartsOrg#2894)
  bump pod version
  chart views now use open legend renderer property instead of internal one (ChartsOrg#3149)
  Fix axis label disappear when zooming in deep enough (ChartsOrg#3132)
  added DataApproximator+N extension (ChartsOrg#2848)
  Minor cleanup to Highlighter types (ChartsOrg#3003)
  Refactored ChartUtils method into CGPoint extension (ChartsOrg#3087)
  Update ViewPortHandler.swift (ChartsOrg#3143)
  add option to build demo projects unit tests on iOS (ChartsOrg#3121)
  Replaced relevant `ChartUtils` methods with `Double` extensions (ChartsOrg#2994)
  Update 4.0.0 with master (ChartsOrg#3135)
  Removed redundant ivars in BarLineChartViewBase (ChartsOrg#3043)
  fix ChartsOrg#1830. credit from ChartsOrg#2049 (ChartsOrg#2874)
  Makes ChartsDemo compiling again (ChartsOrg#3117)
  Fixed using wrong axis (Issue ChartsOrg#2257)
  Removed methods and properties deprecated in 1.0 (ChartsOrg#2996)
  for ChartsOrg#3061 revert animationUpdate() and animationEnd() not trigger crash if subclass does nothing
  The backing var is not necessary. (ChartsOrg#3000)
  ...

# Conflicts:
#	Source/Charts/Data/Implementations/Standard/LineChartDataSet.swift
#	Source/Charts/Highlight/BarHighlighter.swift
#	Source/Charts/Renderers/BarChartRenderer.swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
More Swift
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants