-
Notifications
You must be signed in to change notification settings - Fork 117
Conversation
@cpickhardt15 Hi there, could you please remove support for PHP 7.1 in the travis configuration so we can merge it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cpickhardt15 what i meant was removing support for php 7.1 in travis meaning this file https://github.com/joselfonseca/laravel-api/blob/master/.travis.yml#L4 and in composer.json https://github.com/joselfonseca/laravel-api/blob/master/composer.json#L16 now it should be ^7.2
Hope that helps! Slight confusion, but hopefully this is what you meant. @joselfonseca |
@@ -13,7 +13,7 @@ | |||
} | |||
], | |||
"require": { | |||
"php": "^7.1.3", | |||
"php": "^7.2", | |||
"dingo/api": "^2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to upgrade to Laravel 6, i suggest to change laravel/framework to "^6.0", but also dingo/api to "^2.4", laravel/passport to "^7.5", spatie/laravel-permission to "^3.0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, beyondcode/laravel-dump-server is deprecated for facade/ignition "^1.4"
Finally, i also suggest phpunit/phpunit to "^8.0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, At first i thought it was just a composer update, but if this is for the issue #47 then the actual update is to Laravel 6 which means updating all the packages and updating the actual app directory and config files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are submitting this PR for this issue #47 then what is needed here is to update thee Laravel version to 6 and all of the packages, config files and app directory if needed. Otherwise I can merge this a a patch release, please let me know.
@cpickhardt15 I've changed the base to the 7.0 branch since that is the one this PR will apply, the master branch now is on laravele 6. thanks! |
No description provided.