Skip to content
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

adding compatibility with laravel 5.8 #344

Merged
merged 1 commit into from Apr 23, 2019
Merged

Conversation

seedgabo
Copy link
Contributor

using Events::dispatch instead of Events::fire to fix support with laravel 5.8

Copy link

@rockfreak rockfreak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed to work

@tabacitu
Copy link
Contributor

tabacitu commented Apr 9, 2019

@duellsy I know the package is officially abandoned now - sad to hear but I totally understand it, I know how time-consuming it is to maintain packages. But :-)

Could you please merge & tag this one PR, so that it works on Laravel 5.8 too? This should give your users time to migrate to a different auditing package. Or you the time to find a different maintainer. Will you consider it please?

Cheers! Thanks for a great package. Sad to hear it's no longer feasible for you to maintain it.

@tabacitu
Copy link
Contributor

tabacitu commented Apr 9, 2019

Feel free to add me as maintainer to merge this, if you can't do it yourself. I do not intend to maintain the package, but I do intend to make it work on L5.8 so people have more time to choose alternatives.

@duellsy
Copy link
Member

duellsy commented Apr 12, 2019

happy to merge if it can be confirmed that this will either be backwards compatible, or at least if composer.json is updated to ensure people using older versions of laravel aren't impacted

@borisson
Copy link

This can be committed as-is because the ::fire method was already an alias for ::dispatch in earlier versions of laravel (namely 5.4+).

The dispatch method was added in 5.4 and the fire method was removed, without a proper deprecation path in 5.8. There was a 2 year period in which both fire and dispatch existed but fire was just an alias.

So I think we need to update this PR to also include a change in composer.json to say that this requires laravel/framework: ~5.4

@seedgabo can you please make that change in the pull request so that @duellsy can merge this branch, allowing people using backpack to update to the latest version of laravel without too much pain.

@duellsy
Copy link
Member

duellsy commented Apr 22, 2019

@seedgabo following up on this, if you can make the updates as above, I'm happy to merge this in, and will tag as a new release, so things don't break for existing users with older Laravel installations

@tabacitu
Copy link
Contributor

Exactly. What @borisson said :-) It's been an alias since L5.4.
Sure, here you go @duellsy - check out PR #347

@duellsy duellsy merged commit 5ae7095 into VentureCraft:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants