txbm / angular-chartjs

Directive set for the ChartJS library. Supports data bindings and attribute-level specification for chart specific options. The only Angular ChartJS module that supports ALL chart options as HTML5 attributes :)
MIT License
146 stars 37 forks source link

Issues installing via Bower and Grunt's wiredep #15

Closed JAAulde closed 10 years ago

JAAulde commented 10 years ago

Running the command

bower install --save angular-chartjs

Adds

"angular-chartjs": "~0.1.1"

to my bower.json. The code which is retrieved by that version includes a bower.json file which sets the main property as follows:

"main": "angular-chartjs.js"

It should be:

"src/angular-chartjs.js".

Grunt's wiredep task uses the main property to determine the path to the files which should be added to my project's script includes. Since the path is wrong in the distributed version, this breaks the site.

I noticed that your bower.json in Github is correct, but I do not see any available releases to know what version number to request bower fetch in order to get the proper file.

surajajency commented 10 years ago

it should be:

"dist/angular-chartjs.js"

JAAulde commented 10 years ago

dist/ is new since I opened this issue. I'm not sure whether bower should point at that or not, though. I don't often see dist in a repo--that's usually something created by a build tool.

txbm commented 10 years ago

Hello gentlemen,

I am in the middle of creating a new release for this library that compatible with the new versions of ChartJS. What you see here is a work in progress (although the code is now more or less stable). I need to release it properly and write some basic tests.

@JAAulde As far as having dist/ in the repo goes, I generally don't include it for repos that I know will only be used as standalone NodeJS packages, especially on the server / application side. It is automatically created by the GulpJS build tasks.

The reason I DO however include it for packages like this one (that are client-side only) is because many people (using Bower or otherwise) like to have quick and easy access to the built files without having to do any extra work for including the package into their dependency structure (in the HTML).

While this can cause merge conflicts from time to time with many collaborators, it is usually fairly trivial to work out.

I will update Bower in line with the coming release. If not today, then over this weekend.

JAAulde commented 10 years ago

Hi Peter, thanks for your work and the info. Please don't think I was criticizing the presence of dist/ in the repo, I was just talking through where the main property ought to point and what I'm accustomed to seeing. I have one distributed browser-only library and some others in the works and have considered putting a dist/ in the repo as well.

Cheers, Jim

txbm commented 10 years ago

@JAAulde Oh yeah no worries I just wanted to make it clear what the purpose of it was, no criticism suspected :)

txbm commented 10 years ago

The versions and paths have been updated.