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

Composer support #235

Merged
merged 4 commits into from
Nov 27, 2014
Merged

Composer support #235

merged 4 commits into from
Nov 27, 2014

Conversation

thormeier
Copy link
Contributor

This PR adds support for installing jPlayer via composer to simply install and use it and keep it updated in Symfony2 applications and other projects that use composer.
Also added a short installation guide for now. If you merge this, you might want to register jPlayer on Packagist to get rid of the "repositories": {} part that is currently needed to install it.

@thormeier
Copy link
Contributor Author

I'd be happy if someone could give some feedback on this one whether it does make sense or not.

@Afterster
Copy link
Contributor

+1, I think jplayer should have composer support. Hope it will be merged.

@tabernicola
Copy link

+1, now composer is a standar for php developers

@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

Hello @thormeier would you please sign the CLA and I will look into merging this PR.

I'll need to check that my structural changes to the repo have not broken your composer config files... I suspect that it will break your personal composer install though when you update. I made a rather large error with the versioning system in the repository that affects bower and probably would affect this too.

But you sign the CLA, and I should be able to correct the paths and such while performing the merge. Otherwise, I'll be sure to ask here for some help.

@thormeier
Copy link
Contributor Author

Hi @thepag,
signed the CLA. In case you need any help with the composer config, I'd be happy to help out!

@thormeier
Copy link
Contributor Author

Also, I'm moving the requiring of JQuery to a suggestion part, since one would not want to force devs to use the component version if they already have a CDN version or similar installed. Will make the last changes in about 2 hours, if that is ok.

@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

Thanks. I'll be looking at this tomorrow now. I did a round of merges today that I'd call patches. This is technically "new feature" and will go into dev branch ready for 2.9. I intent to get 2.9 out soon though.

Added more information and moved jquery to suggests
@thormeier
Copy link
Contributor Author

Updated composer.json to have more information and moved jQuery 1.7.2 to suggestions instead of requirements. Can be reviewed again now, thank you very much!

@thepag
Copy link
Contributor

thepag commented Nov 21, 2014

Cheers for updating the config. This path here looks obsolete "popcorn/player/*.js", and should be "dist/popcorn/*.js", for the .min.js and .js but i can correct that.

Doing this one next.

@thepag
Copy link
Contributor

thepag commented Nov 21, 2014

This PR has been merged into the dev branch. I corrected the script and file paths, since they were obsolete. Review the dev branch to see it.

This PR will auto-close when dev gets merged with master.

@thepag thepag merged commit 45b0089 into jplayer:master Nov 27, 2014
@thepag thepag mentioned this pull request Nov 28, 2014
@thepag
Copy link
Contributor

thepag commented Nov 28, 2014

Done.
https://packagist.org/packages/happyworm/jplayer

I notice that someone else registered a jPlayer (fork?) under components/jPlayer.

@thepag
Copy link
Contributor

thepag commented Nov 28, 2014

I'm adding a link to the download page... Is this appropriate terminology?

The jPlayer Composer Package at Packagist

@thormeier
Copy link
Contributor Author

Yes, it is, thank you very much! Also you can remove the "repositories" part of the example composer.json in README.md, this is not necessary anymore, since composer will now download the package via packagist.

@thepag
Copy link
Contributor

thepag commented Nov 28, 2014

Readme edit done.
Uploaded the new download page too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants