-
Notifications
You must be signed in to change notification settings - Fork 401
Upgrade to Laravel 7 #581
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
Upgrade to Laravel 7 #581
Conversation
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.
+1
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.
seems to have all that is needed
@MrReeds does @anteriovieira need to approve these changes to merge the PR? |
seems so, only authorized people are allowed |
@anteriovieira Hi, Do you happen to know when you can approve the request? |
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.
TOP
Up please ! |
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 configured Scrutinizer and Travis to my fork with this PR merged and all the tests passed.
https://travis-ci.org/aglipanci/laravel-auditing/builds/659942480
https://scrutinizer-ci.com/g/aglipanci/laravel-auditing/inspections/3567875c-9e77-4844-b7ea-4e23b382960b
Switched to https://altek.gitlab.io/accountant/. It's a fork by the previous maintainer @quetzyg that already supports L7 and stores changes differently. |
Can you elaborate? Is it easy to migrate? |
Hmmm. What's happening here is interesting... 👀 |
@aglipanci sorry I should have been more clear - I'm starting a new project without existing records. If you are already using the Auditing package, migrating will be tricky as the new library digitally signs ledger entries. Edit: more here https://gitlab.com/altek/accountant/-/issues/26 |
Guys, only the Maintainers can approve this. If you are "desperate" to use L7 with this package, use @efriandika solution. Add this to your "repositories": [
{
"type": "git",
"url": "https://github.com/RossAHC/laravel-auditing"
}
],
"require": {
"owen-it/laravel-auditing": "dev-laravel-7"
}, |
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.
LGTM
What's blocking it? |
|
@icaroscherma Sure, I know. My question is "Why the maintainers are pending this PR?" |
This is a good question, since he's commiting non-stop. (He already had 11 commits today) @anteriovieira Aprova o PR aqui jovem =( |
|
Any updates? |
@anteriovieira Is there anything specific that needs to be changed with this PR for it to be approved? If it's because CI checks haven't been reported it might mean there's a need to remove the outdated check in the protected branch settings in the repository settings. |
I've emailed him yesterday and I got no replies so far. :/ |
Teria alguma previsão para o suporte ao Laravel 7? 😃 🍻 Would you have an estimated date for Laravel 7 support? |
@anteriovieira If you are busy, why don't you recruit maintainers from contributors? |
@mpyw I'm not sure "busy" is accurate, maybe a dusty notifications icon? |
@cwilby I see. Anyway, if the author abandon maintaining, it's just a matter of time that a fork will be registered to Packagist... |
@RossAHC Would you consider registering to Packagist if the PR will have been stale for several weeks from now? |
I agree with @cwilby .. |
@efriandika The concern there is creating another fork of the same project. This has been an issue since the original maintainer (@quetzyg) forked this project and made Accountant - which has gained from refactoring this project. Additionally, there is progress on a migration script to convert this package. That being said, would be great to have this branch merged 🤓 |
I wrote him email on 14th, no reply. Mentioned that maybe he could add maintainers if he is too busy |
I just wrote him on LinkedIn. |
Guys I'm sorry about that. I will go back to work on the package again. |
No description provided.