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

Updating 4.0.0 with latest changes in master #3130

Merged
merged 31 commits into from
Dec 24, 2017
Merged

Updating 4.0.0 with latest changes in master #3130

merged 31 commits into from
Dec 24, 2017

Conversation

jjatie
Copy link
Collaborator

@jjatie jjatie commented Dec 24, 2017

No description provided.

maciejtrybilo and others added 30 commits November 2, 2017 17:36
Since the framework is entirely Swift, internal properties do not need to be marked as @objc
in favour of the Swift `is`
* `Animator` is now non-optional in `Renderer` types

* Fixed merge with master
* `ViewPortHandler` is now non-optional in all, if not most cases

* Removed unnecessary `init()`s from `ViewPortHandler` and related

* `Renderer` now has a constant `ViewPortHandler`

* Pulled latest master

* Fixes for PR review

Removed extraneous comment
Removed extra whitespace

* Removed extra new lines
* `ViewPortHandler` is now non-optional in all, if not most cases

* Removed unnecessary `init()`s from `ViewPortHandler` and related

* `Renderer` now has a constant `ViewPortHandler`

* `ChartRenderer`'s must be initialized with a chart

* CombinedChartRenderer gets the same treatment

* Pulled latest master

* Pulled latest master

* Removed unnecessary comment

* Removed unnecessary whitespace
Fix turning off drag in X and Y axes separately.
almsot all cases of fileprivate were meant to be private. If there is a need to use fileprivate in the future, it should be considered then. Not now.
* Fix a bug may cause infinite loop.

* Update BarLineChartViewBase.swift

put nil check first seems better
…t` (#2993)

* Replaced `ChartUtils.Math` in favour of an extension on `FloatingPoint`

Increases readability, and in many cases removes a set of parentheses to ensure order of operations.

* Moved `normalizedAngled` into `FloatingPoint` extension

Improves readability

* Fix after merge

* Pulled latest master

* Renamed deg2rad/rad2deg
* The backing far is not necessary.

Appears to have been a way to expose the var to obj-c like so many others in the framework.

* Corrections for PR

* Pulled master

* count -> endIndex

where appropriate

* fix a typo ifnot -> if not

Just find a typo ifnot -> if not

* Removed redundant notifyDataSetChanged()
revert animationUpdate() and animationEnd()
not trigger crash if subclass does nothing
* Removed methods and properties deprecated in 1.0

* Removed old bridging functions

These were only being used by deprecated methods.

* Pulled latest master
* Use example drawEntryLabelsEnabled instead of removed drawSliceTextEnabled

* Use in example ILineChartDataSet.mode property instead of removed drawSteppedEnabled one
@codecov-io
Copy link

codecov-io commented Dec 24, 2017

Codecov Report

Merging #3130 into 4.0.0 will increase coverage by 0.87%.
The diff coverage is 26.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##            4.0.0    #3130      +/-   ##
==========================================
+ Coverage   19.77%   20.64%   +0.87%     
==========================================
  Files         113      114       +1     
  Lines       13663    15431    +1768     
  Branches        0      256     +256     
==========================================
+ Hits         2702     3186     +484     
- Misses      10961    12208    +1247     
- Partials        0       37      +37
Impacted Files Coverage Δ
...a/Implementations/Standard/RadarChartDataSet.swift 0% <ø> (ø) ⬆️
...ta/Implementations/Standard/LineChartDataSet.swift 29.85% <ø> (+5.65%) ⬆️
...a/Implementations/Standard/PieChartDataEntry.swift 0% <ø> (ø) ⬆️
Source/Charts/Components/YAxis.swift 74.64% <ø> (-5.02%) ⬇️
.../Implementations/Standard/CandleChartDataSet.swift 0% <ø> (ø) ⬆️
...ata/Implementations/Standard/BarChartDataSet.swift 58.51% <ø> (+0.76%) ⬆️
Source/Charts/Highlight/RadarHighlighter.swift 0% <ø> (ø) ⬆️
...rts/Renderers/LineScatterCandleRadarRenderer.swift 13.04% <ø> (+3.95%) ⬆️
...rce/Charts/Renderers/YAxisRendererRadarChart.swift 0% <ø> (ø) ⬆️
Source/Charts/Charts/CombinedChartView.swift 0% <ø> (ø) ⬆️
... and 172 more

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 bc40b16...428843f. Read the comment docs.

@jjatie jjatie merged commit df62114 into 4.0.0 Dec 24, 2017
@liuxuan30
Copy link
Member

@jjatie please try squash the commits into 4.0 as well. Now you have the access, I think we could come up a new model how do we deal with 4.0 refactoring. e.g. do you push all changes into 4.0, and we review once for all, or still we review every PR?

The cons and pros are that, with a huge single PR from 4.0 to master, it would took lot of time to review. With sub PRs, the workload is split, but the review progress may slow you down.

@jjatie
Copy link
Collaborator Author

jjatie commented Dec 25, 2017

We want to squash commits coming from the master too? I assumed I would squash everything except for what was in master.

@liuxuan30
Copy link
Member

the idea is to make the commit logs simple, so the old fashion is we squash into one commit before filing the PR. But now since Github provide such feature, we are not forcing quash on client side anymore.

@jjatie
Copy link
Collaborator Author

jjatie commented Dec 25, 2017

Will do!

@liuxuan30
Copy link
Member

welcome on board :)

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.

6 participants