Skip to content

Laravel 10 support - #59

Merged
danharrin merged 11 commits into
squirephp:3.xfrom
manavo:3.x
Feb 18, 2023
Merged

Laravel 10 support#59
danharrin merged 11 commits into
squirephp:3.xfrom
manavo:3.x

Conversation

@manavo

Copy link
Copy Markdown
Contributor

Hi,

First time contributing so sorry if it's not quite right!

Tried to get everything passing in Actions, and hopefully all the edits are correct. Just trying to add Laravel 10 support.

Happy to make any changes if needed. Also changing some dependencies might mean this has to be a new major version? Either way, I've worked off the 3.x branch.

Thanks!

@aglipanci

Copy link
Copy Markdown
Contributor

@manavo I am afraid your PR will break support for PHP8.0 which doesn't have to be for Laravel 8 and 9.

@manavo

Copy link
Copy Markdown
ContributorAuthor

Laravel 10 has PHP 8.1 as a minimum requirement, so I'm not sure what other options there are?

@aglipanci

Copy link
Copy Markdown
Contributor

@manavo that is true indeed, however the package is support to work for Laravel 8 up to 10, that means that for older versions of Laravel only the older version of the package can be used. Anyway let's see what the maintainers have to say as I haven't actually tested myself.

Great work on the PR anyway!

@manavo

Copy link
Copy Markdown
ContributorAuthor

That's why I'm guessing that it might have to be a new major version. Although Laravel 8 and 9 should work fine on PHP 8.1, right? So maybe that's still an OK middle ground.

@aglipanci

Copy link
Copy Markdown
Contributor

Yes they should work fine with 8.1 but that doesn't mean that everyone is using them with the latest version of PHP though. There might be other reasons that people might be using PHP 8.0. If there will be a major version this should work fine.

Anyway I am just assuming :D

@manavo

Copy link
Copy Markdown
ContributorAuthor

We'll leave those big decisions for @danharrin 😁

@danharrin

Copy link
Copy Markdown
Collaborator

Please retain PHP 8.0 support for now, as we still support Laravel 9

@manavo

Copy link
Copy Markdown
ContributorAuthor

Hi @danharrin, not sure that's possible, with Laravel 10 requiring PHP 8.1?

@danharrin

Copy link
Copy Markdown
Collaborator

Composer will handle it based on the Laravel version.

@manavo

Copy link
Copy Markdown
ContributorAuthor

I updated the composer.json file to allow PHP 8.0, if that's what you meant? And I guess it'll resolve whatever is required according to that?

@aglipanci

Copy link
Copy Markdown
Contributor

@manavo

Copy link
Copy Markdown
ContributorAuthor

Good catch, thanks @aglipanci, just updated the packages too!

Comment thread.github/workflows/tests.yml Outdated
@aglipanci

Copy link
Copy Markdown
Contributor

Awesome @manavo !

@danharrin you can try to run the workflows and review whenever you can :)

@danharrindanharrin added the enhancement New feature or request label Feb 18, 2023
@danharrin
danharrin merged commit e421406 into squirephp:3.xFeb 18, 2023
@danharrin

Copy link
Copy Markdown
Collaborator

Thanks @manavo and @aglipanci!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@manavo@aglipanci@danharrin