-
Notifications
You must be signed in to change notification settings - Fork 340
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 integration #2171
base: develop
Are you sure you want to change the base?
Composer integration #2171
Conversation
Remove dead code as well
Remove the compatibility and custom implementations as after PHP 5.2 the json extension is loaded by default
Thanks, great work, and definitely the path to follow. I can't remember now if we made any changes to bcrypt and/or recaptcha. I know we did some changes to some libraries we use (like openssl-cryptor). JSON is included because json_encode/decode could be disabled in the configuration, not sure how frequent it's to be disabled, but we should start thinking about dropping some (most) of the compatibility and works toward 5.6/7.x. Maybe add more requirements at installation. About recaptcha, we're using the official version from google, why using a third party implementation? |
Hey, thanks, appreciate that! About the ReCaptcha you are right! I didn't think about it. I will update to use the official repository from google: https://github.com/google/recaptcha I was researching about the BCrypt and I found this old Gist: https://gist.github.com/hasantayyar/1306679 I compared the files and they are identical! I think the best solution is to create a repository and maintain the class. If you don't want to support it, I found a project with ~4k downloads https://packagist.org/packages/eddieantonio/bcrypt, We could use it as well. About the compatibility, I think we should try to move fast to work toward PHP >= 5.6. |
@conejoninja I finished moving all the external libraries to Composer. The AjaxUploader I think is a custom implementation you guys did, right? I created a repository, but I can transfer the ownership to OSClass org and update the PR. |
I was thinking, using the composer we might consider to remove the validation of the extensions from the installation, what do you think?
We have this on composer.json |
Thanks for your job, it's really great. We are using a pretty old version of AjaxUploader, before it went commercial/paid and free again. As far as I remember it's not a custom, but it had changed too much from were we picked it that's not easy to replace with a new version. What was custom/modified is the PHP code to handle the files upload server-side, but that shouldn't be in the library. About the requirements during install, we'd left them there for the moment, as most users don't know what composer is and upload their files via FTP, we'll continue to offer a "complete" package with all the required files. |
Hum, got it! An idea could be to update the dependencies during the installation (the osclass web installer). |
Hey, I saw that someone added a lib for encrypt/decrypt using openssl. But there is also a simple one (and good as well), that I believe it works in our case: g4/crypto |
@conejoninja @xiris Any news about it? |
@BLACKLEG I finished the integration, just waiting for the merge from the core team. |
I created a pull request in the composer-installers project to support osclass plugins and themes. |
Added phpunit dependencies with composer. composer.json based in osclass#2171 Added some tests Added travis ci config
This PR aims to move some external libraries from the folder oc-includes to composer.
I added required php-extensions to composer's manifest file and remove some dead code as well.
There is an issue opened #1874 where some users started to discuss the topic.
Almost all external libraries were moved to be managed by composer:
Only one library was excluded from the project because of the native support since PHP >= 5.2:
Tests
Todo
Updates (20/04)
A bit more
The move of oc-includes/osclass will be provided in another PR, as will be huge and require tons of changes. I extracted and added into another repo. I'm doing tons of tests here https://github.com/xiris/osclass-platform/tree/move_osclass_core