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

update the way of embedding Chart.js #356

Merged
merged 1 commit into from Sep 7, 2016
Merged

Conversation

hongbo-miao
Copy link
Contributor

@hongbo-miao hongbo-miao commented Aug 15, 2016

https://github.com/chartjs/Chart.js/releases, check Version 2.2.0 part:

Starting with v2.2.0-rc.1 Chart.js uses a new build system. We no longer include the checked in files (dist folder) in the repository.

https://github.com/chartjs/Chart.js/releases
Starting with v2.2.0-rc.1 Chart.js uses a new build system. We no longer include the checked in files (dist folder) in the repository.

Usage:
https://github.com/chartjs/Chart.js/blob/master/docs/00-Getting-Started.md#bower
@otelnov otelnov changed the base branch from development to feat-rc6 September 7, 2016 11:57
@otelnov otelnov merged commit 5ce6f66 into feat-rc6 Sep 7, 2016
@valorkin valorkin deleted the Hongbo-Miao-patch-1 branch September 8, 2016 13:32
@simonbrunel
Copy link

@hongbo-miao @valorkin this PR introduced a wrong way to include Chart.js and should be reverted.

The release notes actually said:

Starting with v2.2.0-rc.1 Chart.js uses a new build system. We no longer include the checked in files (dist folder) in the repository. When a tagged release is made, a build will occur and will be published on NPM automatically.

Which mean that the dist files was (and still are) in the npm package under the dist folder.

It seems that this change generated lot of confusion for ng2-charts users because it doesn't work to import src/chart.js via a script tag (#526, #679, #832, #969, #1041, etc.). Also, starting 2.8, the src will be removed from the npm package (chartjs/Chart.js#6105): we consider these files private and our users shouldn't rely on it.

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.

None yet

4 participants