Closed cmbirk closed 8 years ago
The recommended path to upgrading Laravel from 4.x to 5.x is to do a fresh install and then copy files over, so this commit is going to be huge.
But, here we go :)
So far it's looking like one big thing affecting us will be the switch to Elixir for asset management. We'll want to decide whether we want to move to this for our asset management or not.
If we ever want to move the angular frontend to it's own project, then it would be best not to mix it in and rely on it. I'll find out soon enough whether we have to use it for frontend assets or not. I'm thinking not at this point, but we'll see.
I believe this is wrapped up, if anyone wants to check out this branch and do a quick run through of features in case I missed anything, it would be appreciated.
https://github.com/opengovfoundation/madison/tree/issue656-laravel5
@krues8dr @cmbirk
Just a few suggestions/thoughts, without much 'internal' knowledge
Api\AnnotationController
instead of AnnotationApiController
?
Not essential but could be nice-to-have:
return $this->belongsTo('App\Doc');
to use the ::class
notation. Doesn't change the function, but easier to spot incorrect references and click-through.
$_ENV['USERVOICE']
, $_ENV['GA']
with env()
helper. Request $request
as param, instead of Input::get()
) or helpers ( abort()
, redirect()
, view()
etc)All of that sounds good to me, @barryvdh - unless @cmbirk or @sethetter have objections, we can open those as discrete tickets? Some of those might be bigger than we can bite off right now, but all would be useful.
@sethetter I'm reviewing all of this now, gimme a bit to chew through everything here this morning.
+1 on all your suggestions @barryvdh. If you can't tell I'm new to Laravel, so I'm all for input on better approaches and packages to use.
As for Elixir, since the front end is all just an Angular application, using a build tool that's separate from Laravel would be ideal, like we're doing now. I think we're best off not using Elixir.
Thanks @krues8dr! Eager to merge it in :)
Sorry, yes, we're actually going to split out the entire frontend in the (hopefully) near future to a standalone repo which will be JS-only, so Elixir does not make sense here.
Okay, doesn't matter for Laravel anyways, and easier because Grunt is already set up :)
If I'm going to suggest packages, I always use my own ;) https://github.com/barryvdh/laravel-debugbar to profile request. This should actually also work with Angular requests, so you can log all the requests that are made via XHR and for example check all queries that are executed in those ajax calls.
https://github.com/barryvdh/laravel-ide-helper Either as package or just grab a pre-generated version, to make Phpstorm (or other IDE) understand Facades.
I think it would also be better to refactor the controllers to use actual namespaces, instead of classmapping that folder directory. And either place the models in app/
without namespace, or app/Models
and use the Models\
namespace. That way the entire app/
folder is properly autoloaded with PSR-4.
@barryvdh yep, I'm on board with all of that. I'm just getting back into PHP so I'll pick up these best practices along the way :)
Models have already been moved to app/Models/
in d2499993665ac32f4acb2f29c490cd672b0278d9.
Thanks for the input!
Yes I noticed you moved them, but you didn't namespace them. That was what I was referring to.
Models namespaced here -> d33abceb0a8bd12b8e29318ffaf43440db1d7215
@sethetter would you break out any of @barryvdh 's suggestions that you didn't implement into separate tickets?
@cmbirk you got it :)
Would like to get this done before we start getting heavy usage from Chicago.
There's an upgrade guide here on the steps required. There will obviously have to be significant testing afterward as well.