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

Add Symfony 7 support ; drop Symfony < 5.4 support and PHP < 8 support #258

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Add Symfony 7 support ; drop Symfony < 5.4 support and PHP < 8 support #258

merged 7 commits into from
Jan 23, 2024

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Dec 24, 2023

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets #257 #234 #256 #235 #251 #252
License MIT

Hi @lsmith77, I know this bundle might be in low-maintenance mode. But adding SF 7 support would be useful:

  • I dropped PHP 7 support and SF 4 support since they are not maintained.
  • I added SF 7 support
  • I update Php-cs-fixer dependency
  • I fixed the tests and solved the phpunit deprecations

@CondonesMix
Copy link

sorry for the question.

It seems to be solved but composer require friendsofsymfony/ckeditor-bundle keeps failing me, has it already been published in the main branch?

Salutations from Barcelona

@VincentLanglet
Copy link
Contributor Author

It seems to be solved but composer require friendsofsymfony/ckeditor-bundle keeps failing me, has it already been published in the main branch?

That's the whole difference between a merged PR and an open PR.

@CondonesMix
Copy link

And is there a way to install a PR that is open like this?

@jcglombard
Copy link

@VincentLanglet Thx for this 👍🏾
@coreteam please merge this PR to let us upgrade to SF7 😉

@avtarsm
Copy link

avtarsm commented Jan 14, 2024

Please merge this PR.

"symfony/property-access": "^4.4 || ^5.0 || ^6.0",
"symfony/routing": "^4.4 || ^5.0 || ^6.0",
"symfony/twig-bundle": "^4.4 || ^5.0 || ^6.0",
"symfony/asset": "^5.4 || ^6.0 || ^7.0",

Choose a reason for hiding this comment

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

Maybe also bump the ^6.0 to ^6.3 at least (or even ^6.4), as <6.3 is unmaintained and 6.3 support ends this month... https://symfony.com/releases#symfony-releases-calendar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously the support was still ^5.0 || ^6.0", even if it was unmaintained so I kept the same strategy ; even if I agree with you.

@Jean-Gian
Copy link

Great PR!
Do you guys think it will be merged and released soon?

@lsmith77
Copy link
Member

I have commit access but I never used the bundle and I am not really using Symfony, so I don't feel comfortable tagging a release. I have tried to find someone with commit access to take this on ..

@Jean-Gian
Copy link

Do you think this project sort of abandoned?
I see this ultra simple PR created over 6 months ago that hasn't been merged #250
It seems there is no activity nor interest by who can merge this

@mhpcc
Copy link

mhpcc commented Jan 23, 2024

The project is definitely abandoned and there's little point in resurrecting it, as it uses CKEditor 4 which has been EOL for a while too. It would be great to get Symfony 7 support though, as it's not trivial to upgrade to CKEditor 5 due to changed licensing.

@Jean-Gian
Copy link

@lsmith77 considering what @mhpcc said, would you mind merging and tagging anyway?

@lsmith77
Copy link
Member

ok .. from looking over the changes, they are all related to CS fixer. so safe enough.

@lsmith77 lsmith77 merged commit 7ce41e1 into FriendsOfSymfony:2.x Jan 23, 2024
5 checks passed
@lsmith77
Copy link
Member

@VincentLanglet did #235 really get fixed with this PR?

@lsmith77
Copy link
Member

OK this is now merged. I will wait a few more days for feedback until I will tag a release.

@Jean-Gian
Copy link

Amazing @lsmith77 ! Thanks a lot!
And thanks again @VincentLanglet for the PR

@mbabker
Copy link
Contributor

mbabker commented Jan 23, 2024

The project is definitely abandoned and there's little point in resurrecting it, as it uses CKEditor 4 which has been EOL for a while too. It would be great to get Symfony 7 support though, as it's not trivial to upgrade to CKEditor 5 due to changed licensing.

@mhpcc That's why I've opened #254 so that the current state of things can be better signposted. This bundle works well enough and given how the last few releases are predominantly Symfony compat related, it clearly doesn't need a lot of maintenance. But, at this point given the state of CKEditor 4, new users installing this bundle also isn't a great recommendation anymore.

@VincentLanglet
Copy link
Contributor Author

@VincentLanglet did #235 really get fixed with this PR?

Reading again the PR, I'm not sure. We can let it open, just in case...

@Gwadas
Copy link

Gwadas commented Jan 24, 2024

for me is not work ..
Your requirements could not be resolved to an installable set of packages.

Problem 1
- friendsofsymfony/ckeditor-bundle[1.0.0, ..., 1.2.1] require php ^5.6 || ^7.0 -> your php version (8.3.1) does not satisfy that requirement.
- friendsofsymfony/ckeditor-bundle[2.0.0, ..., 2.2.0] require php ^7.1 -> your php version (8.3.1) does not satisfy that requirement.
- friendsofsymfony/ckeditor-bundle 2.3.0 requires symfony/asset ^4.4 || ^5.0 -> found symfony/asset[v4.4.0, ..., v4.4.46, v5.0.0, ..., v5.4.31] but it conflicts with your root composer.json require (^7.0).
- friendsofsymfony/ckeditor-bundle 2.4.0 requires symfony/asset ^4.4 || ^5.0 || ^6.0 -> found symfony/asset[v4.4.0, ..., v4.4.46, v5.0.0, ..., v5.4.31, v6.0.0, ..., v6.4.0] but it conflicts with your root composer.json require (^7.0).
- Root composer.json requires friendsofsymfony/ckeditor-bundle * -> satisfiable by friendsofsymfony/ckeditor-bundle[1.0.0, 1.1.0, 1.2.0, 1.2.1, 2.0.0, ..., 2.4.0].

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals

@lsmith77
Copy link
Member

since I have not tagged a release, you need to allow dev dependencies

@VincentLanglet
Copy link
Contributor Author

Or the version dev-2.x as 2.5

@lsmith77
Copy link
Member

alright. time to tag a release.

@lsmith77
Copy link
Member

https://github.com/FriendsOfSymfony/FOSCKEditorBundle/releases/tag/2.5.0

@AirBair AirBair mentioned this pull request Feb 2, 2024
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.

10 participants