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.

Sign up for GitHub

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

Jump to bottom

[2.9] Drop support for: Laravel 6, Laravel 7, PHP 7.2 and add doctrine/dbal ^3 #1114

Merged
merged 3 commits into from
Dec 29, 2020
Merged

[2.9] Drop support for: Laravel 6, Laravel 7, PHP 7.2 and add doctrine/dbal ^3 #1114

merged 3 commits into from
Dec 29, 2020

Conversation

mfn
Copy link
Collaborator

@mfn mfn commented Dec 5, 2020 •
edited
Loading

Summary

Based on the discussion around #1094 (comment)

❗ BLOCKED UNTIL A NEW 2.8 RELEASE WAS MADE AND SOME TIME PASSED (like, a week) ❗

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@mfn mfn self-assigned this Dec 5, 2020
mfn
mfn commented Dec 5, 2020
View reviewed changes
.github/workflows/run-integration-tests.yml
exclude:
- laravel: 8.*
php: 7.2
php: [8.0, 7.4, 7.3]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding 8.0 here is actually a bug fix, it was forgotten in #1106

.github/workflows/run-tests.yml
Comment on lines 26 to 27
- php: 8.0
dependency-version: prefer-lowest
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still can't get rid of it, this still breaks with dbal

@mfn mfn added this to the 2.9 milestone Dec 6, 2020
This was referenced Dec 6, 2020
Closed
Closed
@barryvdh
Copy link
Owner

barryvdh commented Dec 9, 2020

So should we go ahead with this now? Or are there still issues you want to handle before 2.9?

@mfn
Copy link
Collaborator Author

mfn commented Dec 9, 2020 •
edited
Loading

@barryvdh I would wait "a bit" (like, next week maybe?) to gauge any possible breakers. I also want to ping all the people having the Laravel 8 problems which should also be fixed

spawnia
spawnia approved these changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@spawnia spawnia left a comment

Choose a reason for hiding this comment

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

Nice work. I tried this branch in our project, everything is working great.

@mfn mfn marked this pull request as ready for review December 23, 2020 11:12
@mfn
Copy link
Collaborator Author

mfn commented Dec 23, 2020

@spawnia

Nice work. I tried this branch in our project, everything is working great.

Thanks for confirming!

@barryvdh basically it's all ready!

Though #1134 should be merged first and I'll then adapt the changelog changes here before we merge this.

@mfn mfn requested a review from barryvdh December 23, 2020 11:17
pvsaintpe
pvsaintpe suggested changes Dec 27, 2020
View reviewed changes
composer.json
"phpunit/phpunit": "^8.5 || ^9",
"spatie/phpunit-snapshot-assertions": "^1.4 || ^2.2 || ^3 || ^4",
"spatie/phpunit-snapshot-assertions": "^3 || ^4",
"vimeo/psalm": "^3.12"
Copy link

Choose a reason for hiding this comment

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

"vimeo/psalm": "^3.12" => ^3.12 || ^4.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll postpone such changes, it's not focus of this PR.

Rather, I prefer to get rid of it from require-dev, as it doesn't need to be a source dependency, it's just tooling.

Copy link

Choose a reason for hiding this comment

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

Ok

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I already forgot I made a PR to bump it to ^4 => #1115

Btw, no idea why we would support older versions, don't see much gain? Feedback in #1115 please :)

Copy link

Choose a reason for hiding this comment

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

Thanks.

@pvsaintpe
Copy link

@mfn, @barryvdh

We really need PHP 8.0 + Doctrine\Dbal 3.0 support in our project.
When can we expect a release with these changes? I really don't want to make forks and other crutches ..

Thank you in advance.

@barryvdh barryvdh merged commit 64a6b90 into barryvdh:master Dec 29, 2020
@barryvdh
Copy link
Owner

Oke please test this version on your project, I will tag a new release if okay.

@aaronhuisinga
Copy link

aaronhuisinga commented Dec 29, 2020 •
edited
Loading

Tested and working as expected across a few projects I maintain. Thanks for everyone's work on this!

@mfn mfn deleted the mfn-l8-php7.3 branch December 30, 2020 07:26
@mfn
Copy link
Collaborator Author

mfn commented Dec 30, 2020

Tested on a project with dbal 2.12.1 and then also upgraded to dbal 3.0 and generating model phpdoc worked without issues \o/

@barryvdh
Copy link
Owner

Alrighty, tagged 2.9.0 🎉

@DannyvdSluijs DannyvdSluijs mentioned this pull request Mar 12, 2021
Closed
@dependabot dependabot bot mentioned this pull request Mar 16, 2021
Closed
@mfn mfn mentioned this pull request May 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvsaintpe pvsaintpe pvsaintpe approved these changes

@barryvdh barryvdh barryvdh approved these changes

@spawnia spawnia spawnia approved these changes

Assignees

@mfn mfn

Labels
None yet
Projects
None yet
Milestone
2.9
Development

Successfully merging this pull request may close these issues.
5 participants
@mfn @barryvdh @pvsaintpe @aaronhuisinga @spawnia