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

Rereview #154 #187

Closed
tm1000 opened this issue Jul 14, 2021 · 17 comments
Closed

Rereview #154 #187

tm1000 opened this issue Jul 14, 2021 · 17 comments
Assignees
Labels
enhancement New feature or request

Comments

@tm1000
Copy link
Contributor

tm1000 commented Jul 14, 2021

In #154 it was stated that newer versions of ide helper (greater than 2.9.2) would not be supported because they dropped support for laravel 6 and 7. 21 hours ago #185 was merged which removes support for laravel 7 in this package. So isn't that point moot now. This package itself only supports laravel 8 so shouldn't it be made compatible with the newest ide helper if that was the reason holding it back

@tm1000 tm1000 added the enhancement New feature or request label Jul 14, 2021
@mr-feek
Copy link
Collaborator

mr-feek commented Jul 14, 2021

This package supports Laravel 6, the LTS version

@tm1000
Copy link
Contributor Author

tm1000 commented Jul 14, 2021

Ah right

@tm1000 tm1000 closed this as completed Jul 14, 2021
@jpresutti
Copy link

Laravel 6 is EOL as of 11 days ago. @mr-feek I would really like to use this on a project that is using a newer version of the ide helper.

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 15, 2021

@jpresutti laravel 6 is the LTS version, which has bug fixes until September 3rd, 2022

@jpresutti
Copy link

jpresutti commented Sep 16, 2021

@mr-feek

That is incorrect. Bug fix support ended Sept 3rd. It is security fixes only.
Screenshot_20210916-090642_Samsung Internet
Read the docs.

https://laravel.com/docs/6.x/releases

By not having a version line that supports the newer version of ide helper you are locking out a LOT of projects. If you insist on supporting a version that's reached EOL then perhaps you should do a version jump for also supporting the newer version.

@tm1000
Copy link
Contributor Author

tm1000 commented Sep 16, 2021

Agree with this ^

Also he's not the only one to request it working with newer ide helper

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

@tm1000 @jpresutti like I've said a bunch, if you can resolve this issue while still supporting laravel 6 then I will gladly merge a PR

@jpresutti
Copy link

jpresutti commented Sep 16, 2021

@mr-feek did you actually read the part where you are literally wrong about Laravel 6 still getting bug fixes?

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

@jpresutti would you like me to apologize for a typo? I meant security fixes. Are you actually wasting your time being rude to someone who spent hundreds of hours making this open source package, instead of taking the time to propose a fix?

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

@jpresutti fork it, you're on your own for being a jerk to open source maintainers.

@jpresutti
Copy link

It... it was a serious question. That being said, I'm actually wrong. They pushed it back till January even tho 9 is LTS so I'll see myself out.

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

Or you know, spend 20 minutes making a PR to support both versions. You know, read the docs @jpresutti https://www.php.net/

@jpresutti
Copy link

The whole point of things like Laravel 6,7,8 etc is so there can be multiple supported versions. Your "there can be only one version line" approach is costing users the ability to use the library. The same for there being PHP 7.3, 7.4 and 8 all active at the same time. Again, I was being serious and not trying to be a jerk, and I also said I was wrong on the date because they updated one doc and not another, so you can stop with the snark at any time. It's not a good optic

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

PRs welcome

@tm1000
Copy link
Contributor Author

tm1000 commented Sep 16, 2021

Or you know, spend 20 minutes making a PR to support both versions.

I don't think it's possible to do this because the issue is they changed typings in ide helper (they added callable to something that wasn't typed before). This project would have to have two active versions.

Additionally there's an active branch to remove ide helper dependency

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

@tm1000 a simple version_compare and substituting this package's bindings should suffice

@mr-feek
Copy link
Collaborator

mr-feek commented Sep 16, 2021

I started sketching it out, feel free to finish it https:/psalm/psalm-plugin-laravel/tree/support-newer-ide-helper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants