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

Migration to Symfony 4 #618

Closed
chartjes opened this Issue Nov 16, 2017 · 40 comments

Comments

Projects
None yet
6 participants
@chartjes
Copy link
Contributor

chartjes commented Nov 16, 2017

Silex is hitting EOL in June 2018, so I think it's time for us to figure out how to move OpenCFP more towards Symfony, perhaps targeting Symfony 4.

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Nov 16, 2017

Laravel! 😀

Don't have a huge dog in the "framework fight", more or less personal preference, for what that's worth.

@dbu

This comment has been minimized.

Copy link

dbu commented Nov 17, 2017

i'd like to look into porting it to symfony 4 flex. no promises though, i'll try a bit and then report back if i think that i can seriously propose to do it, to check if that is whats wanted.

@chartjes

This comment has been minimized.

Copy link
Contributor Author

chartjes commented Nov 29, 2017

@dbu I definitely think Symfony 4 is the target to move to given how much Symfony stuff we are already relying on. Would greatly appreciate you taking a look

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 1, 2017

Just as a heads up: I've started working on this. I've got the framework + some basic components (webpack-encore, twig, security) started. I'm working on pulling over our CI process and helping some things settle (the divergence between .travis.yml, Makefile, and script/*). I feel like TravisCI should be running (more or less) the same commands we are. If that happens to be script/test, cool. Currently, I've got to remember make cs, script/setup and .travis.yml is doing other things in scripts. Now's probably a good time to clean that up.

Also pulling over all the php-cs-fixer / .codeclimate.yml stuff piece by piece.

I'm also including a simple LEMP stack on Docker that runs OpenCFP. You basically run cfp shell and it builds everything and drops you in a workspace to finish setting up the application.

I think that at some point it is going to be helpful to pump the breaks on PRs, but no need for that right now. I feel like the best option right now is to create a baseline, move over a few things, then we all agree to send PRs to a symfony4 branch to move the rest of everything over. I think once I get the frontend working, complete with signup / login process handled, I'll put the branch up and we can all start migrating things in.

This is also a GREAT time to create a decent service layer and inject dependencies into controllers rather than reaching out to the container constantly. Make it part of the "definition of done" for moving code into the new framework.

@dbu

This comment has been minimized.

Copy link

dbu commented Dec 1, 2017

sounds like a great plan! and glad you are on it. i don't think i would really find time to work on it anytime soon, after all... too much interesting stuff to do, too little time.

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 1, 2017

That is exactly the problem I seem to find myself in pretty much all the time. Cheers!

@chartjes

This comment has been minimized.

Copy link
Contributor Author

chartjes commented Dec 1, 2017

@mdwheele The Symfony project has a page where they trying to put together a small guide to help people migrate. Anything you run into that is difficult should be reported here.

symfony/symfony-docs#8678

@mdwheele mdwheele changed the title RFC: Moving away from Silex Migration to Symfony 4 Dec 4, 2017

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 4, 2017

Hehe, I saw that! The migration is going well, so far. Our approach is very similar to others' in that issue. I'll make sure to report any problems I run into. I think we're actually going to be able to maintain backwards-compatibility, for what it's worth.

I'm scoping this migration to include account management, authentication and talk submissions. I am purposefully leaving out much of the administrative use-cases. This leaves us open to include an open-source bundle for that process or not. I think there are some biz discussions to be had.

It would be convenient to base this branch on a tagged release so I'm not having to manually patch in diffs from PRs to upstream. That said, it's going to be a bit longer before I'd say we need to do that so PRs away!

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 4, 2017

I'm currently doing some research on how a migration path from Silex to Symfony could look like. Since this project is based on Silex and has a well maintained codebase, I gave it a shot. I even made it to a working prototype without the silex/silex package.

If you're interested, I can push it, but most tests are broken and the diff is too large to keep it in sync with changes that have already happened on master. Also, I didn't want to take away the fun of performing that migration. ;-)

If that is of any help and you're interested, I could pretty easily provide you with the following PRs:

  • Controllers refactored to services. This makes then more or less autowirable when switching to Symfony. Also, the routing configuration remains compatible.
  • Middlewares refactored to event subscribers. When migrating to Symfony, those can can be reused without modification.

Both refactorings are imho good preparations for a Symfony migration.

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 4, 2017

@derrabus if you want to push those changes to a branch on your fork, I'd be happy to take a look. I'm already pretty far into the move, myself, but would be interested in comparing.

I wanted also to use this as an opportunity to clean up and not necessarily move everything over 1:1.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 5, 2017

@mdwheele Here you go: https://github.com/derrabus/opencfp/tree/remove-silex

As I said, it's a prototype. I did not care much about the tests, so I might have broken a couple of things. One feature that is definitely gone is the ability to turn off the cache. Since Symfony works with a compiled container and router, it would be pretty pointless to work without a cache.

@localheinz

This comment has been minimized.

Copy link
Collaborator

localheinz commented Dec 5, 2017

@derrabus

Hehe, I remember your talk at SymfonyCon Berlin 2016: https://berlincon2016.symfony.com/speakers#session-1851.

👍

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 5, 2017

Thanks @derrabus! With all that's going on in #850 (or in split PRs; whatever is decided), I think I'm going to hold off moving forward with my branch. If you want to send what you have as a PR, we could work through that. I get about 4-5 hours between 10pm - 3am to work on projects like this and have felt like the last few weeks have been 2 hours of rebasing to make 30 minutes of progress, haha.

If others have time to carry this forward, I'd be happy to help review. If things settle a bit and the migration isn't resolved, I can pick it up then.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 5, 2017

@mdwheele I think this migration should not be done in a big-bang-all-or-nothing-PR. We can do this in small steps that all result in a runnable application. That's why I've submitted that PR: it reduces the coupling with Silex, so the diff for a Symfony 4 migration becomes smaller.

@mdwheele

This comment has been minimized.

Copy link
Contributor

mdwheele commented Dec 5, 2017

I think this migration should not be done in a big-bang-all-or-nothing-PR. We can do this in small steps that all result in a runnable application. That's why I've submitted that PR: it reduces the coupling with Silex, so the diff for a Symfony 4 migration becomes smaller.

Agreed! Just to clarify; don't take my previous comment as "frustrated". It is what it is. I'll never ever say "too many contributors is a bad thing". I just don't want to get in the way 😀. We appreciate the help.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 7, 2017

Seems like briannesbitt/Carbon#1028 is blocking Symfony 4 for us at the moment.

@BackEndTea

This comment has been minimized.

Copy link
Collaborator

BackEndTea commented Dec 8, 2017

On the topic of upgrading to php 7.1, It might be time to give all our dependencies a good look-over, and see if we want to swap any out, not all of them are being tested against php 7.1, some are not even being tested against php 7.0.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 16, 2017

#895 is ready for review.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 16, 2017

So, which libraries do currently block us from upgrading to Symfony 4?

$ composer why-not symfony/config:4
robmorgan/phinx  v0.9.1  requires  symfony/config (~2.8|~3.0)

$ composer why-not symfony/console:4
infection/infection  0.6.2   requires  symfony/console (^3.2)
robmorgan/phinx      v0.9.1  requires  symfony/console (~2.8|~3.0)

$ composer why-not symfony/finder:4
illuminate/filesystem  v5.5.17  requires  symfony/finder (~3.3)
infection/infection    0.6.2    requires  symfony/finder (^3.2)

$ composer why-not symfony/http-foundation:4
illuminate/validation  v5.5.17  requires  symfony/http-foundation (~3.3)

$ composer why-not symfony/process:4
infection/infection  0.6.2  requires  symfony/process (^3.2)

$ composer why-not symfony/translation:4
nesbot/carbon  1.22.1  requires  symfony/translation (~2.6 || ~3.0)

$ composer why-not symfony/yaml:4
infection/infection  0.6.2   requires  symfony/yaml (^3.3)
robmorgan/phinx      v0.9.1  requires  symfony/yaml (~2.8|~3.0)
@localheinz

This comment has been minimized.

Copy link
Collaborator

localheinz commented Dec 16, 2017

@derrabus

What do you think about replacing robmorgan/phinx with doctrine/migrations?

This would also make sense in the light of switching to using doctrine/orm, if we wanted to.

@BackEndTea

This comment has been minimized.

Copy link
Collaborator

BackEndTea commented Dec 16, 2017

For infection we can move to the phar if needed. But i know they are working on being 4.0 compatible.

@localheinz

This comment has been minimized.

Copy link
Collaborator

localheinz commented Dec 16, 2017

How about removing all of the Laravel components?

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 16, 2017

@localheinz Pick the library that works for you. Doctrine Migrations is powerful when used with the ORM. As a stand-alone tool, it's as good as any other migration tool, imho.

I wouldn't worry too much about blocking libraries just yet. Symfony 3.4 and 4.0 have the same feature set, so until May 2018 we would not gain much from upgrading to 4.0 anyway.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 17, 2017

Phinx will support Symfony 4 with its next release, see cakephp/phinx#1224.

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 17, 2017

How about removing all of the Laravel components?

I mean, you could replace Eloquent with Doctrine ORM, Sentinel with Symfony Security, Illuminate Validation with Symfony Validation etc. if you want to be more "standard Symfony". But as long as those libraries and are maintained and solve your problem, I don't see an urgent need to go that way.

@localheinz

This comment has been minimized.

Copy link
Collaborator

localheinz commented Dec 17, 2017

@derrabus

For me it's not about being more standard Symfony. For me it's about being able to properly test components. If I see the effort required to test components using Eloquent models - or any other Active Record model, then I say "thanks, but no thanks".

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 17, 2017

If I see the effort required to test components using Eloquent models - or any other Active Record model, then I say "thanks, but no thanks".

That is a valid point indeed. Since this is imho not related to the Symfony migration, should we open another issue for this discussion?

@localheinz

This comment has been minimized.

Copy link
Collaborator

localheinz commented Dec 17, 2017

@derrabus

Sounds good! 👌

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Dec 28, 2017

There are now stable releases of Infection and Phinx that are compatible with Symfony 4. Now we just have to wait for the Illuminate components (or replace them).

@derrabus derrabus referenced this issue Feb 19, 2018

Merged

Fix: Drop support for PHP 7.0 #1107

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.