Resubmit of #2730

[jjatie]

Resubmit of #2730

[codecov-io]

Codecov Report

Merging #3002 into master will increase coverage by 0.64%.
The diff coverage is 70.31%.

@@            Coverage Diff             @@
##           master    #3002      +/-   ##
==========================================
+ Coverage      20%   20.64%   +0.64%     
==========================================
  Files         113      114       +1     
  Lines       15529    15431      -98     
  Branches      249      256       +7     
==========================================
+ Hits         3106     3186      +80     
+ Misses      12387    12208     -179     
- Partials       36       37       +1

Impacted Files	Coverage Δ
...s/Data/Implementations/Standard/ChartDataSet.swift	37.08% <0%> (ø)	⬆️
Source/Charts/Charts/BarLineChartViewBase.swift	22.56% <0%> (+0.03%)	⬆️
.../Implementations/Standard/ChartDataEntryBase.swift	31.7% <0%> (+23.81%)	⬆️
Tests/Charts/EquatableTests.swift	100% <100%> (ø)
...Data/Implementations/Standard/ChartDataEntry.swift	36.73% <63.63%> (+30.32%)	⬆️
Source/Charts/Highlight/Highlight.swift	41.33% <81.81%> (+41.33%)	⬆️

---

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 8b18e76...0ef8aa1. Read the comment docs.

[pmairoldi]

With the revert to isEqual seems like methods are just moved around. Is there any meaningful change anymore?

[jjatie]

• Use of == in the framework as per Swift guidelines
• removed the == implementations as noted by the Objective-C/Swift interoperability guide
• simplified the isEqual implementations for ChartDataEntryBase and Highlight
• removed dependence of the super class in the ChartDataEntry isEqual implementation

[jjatie]

@petester42 Any further feedback on this one or can it be merged?

[liuxuan30]

@jjatie we still need a review process for what coming into master.

[jjatie]

Agreed. This was a PR that has had feedback back and forth since August. On the PR this one replaced, we agreed this would be merged in after the Swift 4 migration. I made the adjustments requested from the review, and this has just been sitting here since.

This PR removes a few mistaken == implementations, and takes advantage of the Swift Equatable protocol without changing any implementations. Seemed safe to merge after all this time.

[liuxuan30]

I made the same template as yours for combined chart test. Shall we include the license info just like source code template, or test file is fine with default Xcode template?
The old test file does not have a template as well

[jjatie]

I guess we should be putting in the license info to be consistent. Do we keep it the same, but whose name do we use?

[liuxuan30]

I would just use the same template as source code. I don't know the rule of names.
Personally I just use Copyright 2015 Daniel Cohen Gindi & Philipp Jahoda, and I don't need to specify who created the file.
I think this is a good question, as I saw some files with yours template:

//
//  DemoBaseViewController.swift
//  ChartsDemo-iOS
//
//  Created by Jacob Christie on 2017-07-03.
//  Copyright © 2017 jc. All rights reserved.
//

I guess we need an unified one. @petester42 @danielgindi @PhilJay insights?

[jjatie]

Yea, I just use the Xcode default.

[danielgindi]

Just one thing: Please conform to the overall style of the library.
I created this library with braces in new lines as I believe it makes for the cleanest and most readable code, and when there's mixed use of braces up and down - it creates a braces hell.

In any codebase that I participate - I use the style of that codebase, braces up or down, spaces or no spaces, var naming etc.

[liuxuan30]

@danielgindi how about the file template about license and names?