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

added support for laravel 9 #2

Closed
wants to merge 6 commits into from

Conversation

stojankukrika
Copy link

No description provided.

@ilzrv
Copy link
Owner

ilzrv commented Jun 14, 2022

Thanks for contribution. Can you add Laravel 9 to test suite?

@stojankukrika
Copy link
Author

I will try out :)

@stojankukrika
Copy link
Author

@ilzrv added :)

@ilzrv
Copy link
Owner

ilzrv commented Jun 15, 2022

Why did you make 2 PR? You should do it in one place.

@stojankukrika
Copy link
Author

stojankukrika commented Jun 15, 2022

fixed

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
@stojankukrika
Copy link
Author

stojankukrika commented Jun 16, 2022

But looks like tests also needed to be refactored...
https://github.com/stojankukrika/laravel-slow-query-detector/runs/6915560450?check_suite_focus=true
now package works with closure not with string anymore.

Can you help with it?

@ilzrv
Copy link
Owner

ilzrv commented Jun 20, 2022

@stojankukrika You can try to change the version of this dependency like this "^1.6" || ^2.0

@stojankukrika
Copy link
Author

stojankukrika commented Jun 23, 2022

It is like that, take a look.
I think tests need to be rewritten to use that new way (version 2. Old customers of the query detector package can use the old versions but new users can new version, that looks OK to me. What do you think?

@ilzrv
Copy link
Owner

ilzrv commented Jun 23, 2022

I'll think about it. In the second version of the package, we can get away from supporting EOL versions of Laravel.

@stojankukrika
Copy link
Author

stojankukrika commented Jun 23, 2022

Yes. Old version of this pagkage can use old versions of laravel and new versions of laravel can use a new version of package. Like this one laravel-datatables

@ilzrv
Copy link
Owner

ilzrv commented Jun 25, 2022

@stojankukrika Completely updated the library, you can check PR: #4

@stojankukrika
Copy link
Author

looks ok to me :)

@ilzrv
Copy link
Owner

ilzrv commented Jun 29, 2022

Closed in favor of #4

@ilzrv ilzrv closed this Jun 29, 2022
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