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

Alternative for SPM dynamic linking #4478

Merged
merged 3 commits into from
Oct 8, 2020
Merged

Conversation

alexruperez
Copy link
Contributor

@alexruperez alexruperez commented Sep 24, 2020

We need dynamic linking for this dependence.


This change is Reviewable

@jjatie
Copy link
Collaborator

jjatie commented Sep 25, 2020

@alexruperez Could you elaborate on why you need dynamic linking for Charts? I am curious what the use case is.

@alexruperez
Copy link
Contributor Author

@jjatie we have Charts integrated in a dynamic framework through Swift Package Manager, when trying to integrate this framework into other dynamic frameworks and in our app, being static, it gives duplicated symbols error.

@jjatie
Copy link
Collaborator

jjatie commented Sep 30, 2020

Makes sense. You'll need to ensure CI is passing.

@alexruperez
Copy link
Contributor Author

The CI is failing, but not from this MR code. 😞

==> java has been moved to Homebrew.
To uninstall the cask run:
brew cask uninstall --force java
==> Installing java...
Warning: openjdk 13.0.2+8_2 is already installed and up-to-date
To reinstall 13.0.2+8_2, run `brew reinstall openjdk`
The command "brew update" failed and exited with 1 during .

Copy link

@danicoello danicoello left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #4478   +/-   ##
=========================================
  Coverage          ?   41.10%           
=========================================
  Files             ?      124           
  Lines             ?     9423           
  Branches          ?        0           
=========================================
  Hits              ?     3873           
  Misses            ?     5550           
  Partials          ?        0           

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 84fbdcf...8f82705. Read the comment docs.

@alexruperez
Copy link
Contributor Author

@jjatie @danielgindi @danicoello @liuxuan30 ready for merge! 😃

@jjatie jjatie merged commit ae78bad into ChartsOrg:master Oct 8, 2020
@alexruperez
Copy link
Contributor Author

alexruperez commented Oct 16, 2020

When will be available a new release (tag) with this changes? Thanks.
CC: @jjatie @danielgindi @danicoello

@liuxuan30
Copy link
Member

emm, not sure if we want to release a minor version for this. We are planning a 4.0 release soon, at least like a pre release, as it's a very big change and upgrade

ccworld1000 added a commit to ccworld1000/Charts that referenced this pull request Oct 26, 2020
Alternative for SPM dynamic linking (ChartsOrg#4478)
mosaic-engineering pushed a commit to mosaic-io/Charts that referenced this pull request Dec 3, 2020
zebraciam added a commit to zebraciam/Charts that referenced this pull request Jul 6, 2022
* master:
  update changelog.
  Fixed incorrect guard return statement when rendering limit lines (ChartsOrg#4563)
  Fix bounds checks on binary search (ChartsOrg#4577)
  Added SPM build action (ChartsOrg#4576)
  Replace FBSnapshotTestCase with pointfree/swift-snapshot-testing (ChartsOrg#4574)
  Import swift algorithms (ChartsOrg#4497)
  ChartViewBase cleanup (ChartsOrg#4537)
  SPM GitHub Action (ChartsOrg#4553)
  Algorithm updates (ChartsOrg#3638)
  Added SPM Install section
  Update README.md
  Fix missing drawIconsEnabled parameter initialization in the copying constructor of the ChartBaseDataSet (ChartsOrg#4424)
  Resolve conflict for 4.0 branch and master (ChartsOrg#4456)
  Alternative for SPM dynamic linking (ChartsOrg#4478)
  3.6.0 changelog

# Conflicts:
#	Source/Charts/Renderers/LineChartRenderer.swift
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.

5 participants