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

Fix deprecated #240

Merged
merged 5 commits into from
Feb 13, 2022
Merged

Fix deprecated #240

merged 5 commits into from
Feb 13, 2022

Conversation

TiiFuchs
Copy link
Contributor

@TiiFuchs TiiFuchs commented Feb 13, 2022

I took a look at the backtrace and since it's using the debug_backtrace method from PHP itself and apparently $frame->class can be null there, I figured it's the best way to make this more robust and only look at the frames that have a class to begin with.

@TiiFuchs
Copy link
Contributor Author

This fixes #239

@freekmurze
Copy link
Member

Seems like the tests are failing for this one. Could you take a look at that?

@TiiFuchs
Copy link
Contributor Author

Looks like dependency issues with PHP 8 and 8.1... I'll take a look into it.

@TiiFuchs
Copy link
Contributor Author

The problem seems to be a dependency on the package itself.

  • laravel-ray requires orchestra/testbench 7.x in the tests
  • testbench depends on laravel-ray 1.28

As far as I understand it the development branch does not satisfy this ^1.28 requirement and therefore it fails.
See https://getcomposer.org/doc/articles/troubleshooting.md#dependencies-on-the-root-package

But ... I can't quite wrap my head around why the Laravel 8 tests pass without the same problem. laravel-ray requires testbench 6.x which requires laravel-ray ^1.26.2.

composer docs recommend defining a branch alias

@TiiFuchs
Copy link
Contributor Author

Oh one sec... I accidentally commited the changes that were made by the tests.

@TiiFuchs
Copy link
Contributor Author

As I understand it and according to my local tests, this should work now...

@TiiFuchs
Copy link
Contributor Author

Apparently not…

but this is beyond my understanding of composer and it has nothing to do with the problem and fix from the beginning.

Maybe someone with a deeper understanding of those root package dependency stuff can have a look at it…?

@freekmurze freekmurze merged commit 6465e1f into spatie:main Feb 13, 2022
@freekmurze
Copy link
Member

I'll merge this in optimistically so you can already enjoy your fix. I'll take care of tests later. Thanks for your work on this.

@TiiFuchs TiiFuchs deleted the fix-deprecated branch February 13, 2022 16:38
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.

2 participants