-
Notifications
You must be signed in to change notification settings - Fork 21
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 #5
Conversation
Required to autoload the API wrapper
bump |
Definitely useful, please merge the pull request :) |
+1 |
@camfindlay @DamienMetzger @gaillafr thanks for the PR and the feedback! I'm a tiny bit nervous about this, because we've always considered this a piece of sample code, rather than an official library. We're huge composer fans otherwise, so I like the idea of making this accessible to more users. My only concern is making it an official library (installed via composer) now, before we've considered its implications. Glad to do it as a stopgap, if people are comfortable with the interface changing quite a bit as it evolves. Thoughts? (And again, thanks for engaging!) |
Here is my suggestion: Accept the composer.json file (it only shows up in packagist when you actually submit it), as a stop gap until you are happy to submit to packagist user can simply put a "repository" config in their composer.json. I'd also suggest adding an open source license something like a BSD or MIT (note if you're worried about any liability or risk these licenses mean you retain copyright, license a bundle of rights for others to use/modify/redistribute the code and the absolve your company of any legal obligation). Happy to help if you need to understand more about open source licenses. Make sure and tag the current state of this code as 0.1.0 and as you do major refactors of the code try releasing version based on semantic versioning http://semver.org Open source users are pretty tolerant of changes as long as you're good with tagging your versions and the valuable bit is actually progressing the work transparently here on GitHub so those of us using the classes can help with peer review/catching bugs etc. |
I agree with every single points of @camfindlay answer :) There are no implications as long as it is not submitted to packagist. |
Hey @andyfowler just following up on this. I don't think there is any major implication for accepting the composer file as mentioned above. Unless you actively submit this to Packagist it won't be listed there and having a composer file makes this still very useful for developers. The php wrapper is useful and it makes sense to be able to use it directly from GitHub via Composer rather than making a copy paste of this or a fork (which is what I'm currently doing). I'd suggest simply add a permissive license to the repo and accept the composer pull request. A license such as the 3-clause BSD legally negates any warranty if there is a concern about commercial risks or obligations from Nutshell by having others use this code (even if it just a sample it's conceivable that someone can copy/paste this into their application). Happy to have further discussions with you around open sourcing if needed and awesome that you guys have a GitHub and release your API wrappers here. 👍 |
Having the package fully unit tested (ideally through an automated build process like Travis CI) would also contribute significantly to the project's trustworthiness. For my company, not being install-able via composer and not being fully tested are both automatic deal-breakers. Nutshell itself is off the table for any of our client projects based on the weakness of the first-party (PHP) API library. I realize it's a lot of work to maintain professional quality libraries in a lot of (probably unfamiliar) languages, but when I'm reviewing CRM solutions these things keep otherwise viable packages off the short list. I'd love for that to be different a year from now. Good luck, this is a good first step! 👍 |
This allows these classes to be included into projects via the PHP dependancy manager Composer and be autoloaded.