-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add composer.json to let PHP developers to keep track of chosen on packagist. #2803
Conversation
Yeah! @pfiller, @kenearley, ... please merge it. |
I have no opinion here, didn't know composer was being used for non-php projects. Also I don't think I can help out with creating an account on Packagist for this package, since I'm not the owner. So @pfiller, what do you think about this? |
Literally have never heard of any of this, but I haven't worked with PHP in many, many years. @koenpunt Is this something you'd recommend doing? We've discussed trying to get into assorted other package managers over the years, but that seems like an uphill battle that will never be won, since they seem to pop up constantly, and we've so far been conservative about littering .json files all over Chosen to hit all of them. But if this is the one to do for a wide swath of people (and has proven itself to have legs and won't be replaced in 3 months), we can look into it. /cc @stof who I think does a lot of PHP. |
@tjschuck I'm quite against registering non-PHP code on Packagist, as it then leads to cases where people are asking us to add features in composer which are totally useless for PHP packages. |
and if we do it, the repo being registered on Packagist must indeed be the one containing built code |
I think that |
Please, take a look on examples below. These are all javascript libraries, ready to be part of other libraries, CMS and frameworks. https://packagist.org/packages/twbs/bootstrap |
I forgot to say about built code. Packagist can read github releases page, so if your library is ready to use after download, probably is ready to join packagist also. No needs to modify your pipeline. |
I'm not against, but still I'm not the one that can set this up on packagist I believe. Also the indentation in the |
@fabiomontefuscolo Composer is not able to use custom artifacts attached to github releases. So it would still have to use the chosen-package repository storing the build output |
Ok, first thing is about compose.json it self. I fixed white spaces, but build is broken now. Second, I think it is ok composer fetch release from chosen-package, because it is better than handle several packages managers for a single software. On this way, other PHP developers can create their pipelines using just composer.json and avoid npm, yarn, bower, grunt, gulp and other tools that emerge every day. |
but then, this PR is adding the file in the wrong place, as it adds it in the root of the chosen repo, not in the location being used to build the content of chosen-package. |
Is this the solution? |
I don't understand where is problem. If you add @stof I don't agree. It's on right place. Must be in root. |
@landsman but it must be at the root of chosen-package, not of chosen. With the existing location, it won't be in chosen-package at all |
@landsman Please note that though @stof doesn't work at Harvest, he is a maintainer of Chosen. He's also correct that |
Packagist don't fetch released package but whole repository. |
@landsman https://github.com/harvesthq/chosen-package is the source repo for distribution, so packagist should always refer to that one, instead of the main repo (this one) |
Thanks @koenpunt I got it. I sent new pull request with these changes. |
I have created a |
Also, I'm going to close this in favor of #2988 — please continue any discussion over there. |
Hello,
This refers to issue #2802.
If you accept this PR, there are another 3 steps. Unfortunately, I can't do it for you.
I'm unsure, but I think this file should live in chosen-package.
Thank you!