-
Notifications
You must be signed in to change notification settings - Fork 146
Laravel 6 support? #118
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
Comments
Wondering the same thing. @ShawnMcCool @mitchellvanw |
I just merged this: #117 Could you test it on dev-master and verify that it's working for you? |
Nope! Got this error today after upgrading to l6.
Details: Error on Flareapp |
@jetwes |
@ShawnMcCool Any updates on the release for Laravel 6? |
@ShawnMcCool I have just been caught out by trying to update to Laravel 6.0. I need to do this as the new Credit Card laws are now in place so I need Cashier 10. Any update? I may hav to remove your package in the meantime. Thanks. |
I'm waiting for someone to submit a pull request. Once it's submitted I'll be able to merge it in. |
@ShawnMcCool I appreciate you are waiting for them but any idea how many days that might take? Time is of the essence for me on this. :-) |
How many days it'll take for someone to post a fix? I couldn't possibly guess. Could you post a PR? You appear to know exactly what needs to be done. You could be a perfect contributor to resolve this. This is not a project that I actively develop on. This is completely updated by contributors such as yourself. @kundancool I noticed that you seem to have a solution in mind. Perhaps you'd make the pull request? |
Oh I see, I didn't realise that. If I have time I can look at this but hopefully someone can fix it before then. |
If we edit this line https://github.com/backup-manager/laravel/blob/master/src/GetDatabaseConfig.php#L42
This error was gone (I am using MariaDB 10.2).
But removing this might will affect a problem for mysql 8. This was added on this PR #113 by @robbielove. @robbielove would you mind to test it on mysql 8? |
I don't use mysql 8 sorry, but as I stated in the original PR - this needs to be an optional parameter that can be passed through rather than a hardcoded parameter - which I simply did to move past that issue - I didn't expect it to be merged without modification. For reference - I use mysql 5.7.23 in dev and maria db in prod: Server version: 10.3.17-MariaDB-1:10.3.17, I experience this issue in prod however I don't need the ability to actually perform a backup currently. |
Noted @robbielove. Thanks for the insight. I will try to work on the PR to make it optional on the config file. Thanks. |
I've merged the PR, let me know if I can help further. |
Thanks @ShawnMcCool, would you mind to add new tag for latest master branch? |
Sorry for the process but could someone install dev-master and verify that it's working? Is there anything that we need to add to the documentation about the new changes? I can merge later today if someone helps me verify dev-master. |
Got it @ShawnMcCool, I will try the dev-master, then inform you after that. |
Hi @ShawnMcCool, I did verify the dev-master (c78f0d4) on these databases:
The backup and restore feature works with no problem. 👍 May we proceed for a new release tag? |
I have also tested backups only; with Mysql 5.7.23 and MariaDB: Server version: 10.3.17-MariaDB-1:10.3.17 |
https://github.com/backup-manager/laravel/tree/1.4.0 Thanks so much for all of your work here. Work like this is how this package gets updated. Please let me know anything further must be done. |
Yw @ShawnMcCool, nice to contribute in this repo. I use this package a lot. |
Having dependencies issues with Laravel 6
The text was updated successfully, but these errors were encountered: