Skip to content

Conversation

driesvints
Copy link
Member

This pull request includes the changes for upgrading to Laravel 5.8. Feel free to commit any additional changes to the shift-19040 branch.

Before merging, you need to:

  • Checkout the shift-19040 branch
  • Review all pull request comments for additional changes
  • Update your dependencies for Laravel 5.8
  • Run composer update (if the scripts fail, add --no-scripts)
  • Thoroughly test your application (no tests?)

If you need help with your upgrade, check out the Human Shifts. You may also join the Shifty Coders Slack workspace to level-up your Laravel skills.

@driesvints
Copy link
Member Author

driesvints commented Sep 28, 2019

❌ Shift could not upgrade the following configuration files since they differed from the default Laravel version. You will need to compare these configuration files against the Laravel 5.8 configuration files and merge any changes:

  • config/app.php
  • config/database.php
  • config/logging.php
  • config/mail.php
  • config/queue.php
  • config/services.php

@driesvints
Copy link
Member Author

⚠️ Laravel 5.8 introduced a set of AWS environment variables to help consolidate configuring the various AWS services. You should rename the following native environment variables to their new names. While you don't have to rename any custom environment variables you created for AWS services, you should consider following the new naming convention.

  • SQS_KEY to AWS_ACCESS_KEY_ID
  • SQS_SECRET to AWS_SECRET_ACCESS_KEY
  • SQS_REGION to AWS_DEFAULT_REGION
  • SES_KEY to AWS_ACCESS_KEY_ID
  • SES_SECRET to AWS_SECRET_ACCESS_KEY
  • SES_REGION to AWS_DEFAULT_REGION

@driesvints
Copy link
Member Author

❌ Laravel 5.8 has a development dependency for beyondcode/laravel-dump-server of ^1.0. You have a modified dependency for beyondcode/laravel-dump-server in your composer.json. You should review your dependency and ensure it meets the new version requirement.

@driesvints
Copy link
Member Author

❌ Laravel 5.8 has a development dependency for filp/whoops of ^2.0. You have a modified dependency for filp/whoops in your composer.json. You should review your dependency and ensure it meets the new version requirement.

@driesvints
Copy link
Member Author

❌ Laravel 5.8 has a development dependency for fzaninotto/faker of ^1.4. You have a modified dependency for fzaninotto/faker in your composer.json. You should review your dependency and ensure it meets the new version requirement.

@driesvints
Copy link
Member Author

driesvints commented Sep 28, 2019

⚠️ Laravel 5.8 changed several of the core contracts with new implementations and methods. You should review the Upgrade Guide for more detail on these respective changes:

Shift found references to these contracts within:

  • app/Http/Controllers/Auth/RegisterController.php
  • app/Http/Requests/Request.php

@driesvints
Copy link
Member Author

⚠️ Laravel 5.8 changed the TTL for the Cache methods from minutes to seconds. Shift automated this change, however, given the dynamic nature of PHP your application may still contain some TTL arguments in minutes.

While your application will still run, this would lead to early expiration for your cached values. You should review your caching calls to ensure the TTL was upgraded to seconds.

@driesvints
Copy link
Member Author

ℹ️ Laravel 5.8 changed the minimum requirement for password length from 6 characters to 8 character. To preserve the 6 character limit requires you to create a custom PasswordBroker. If possible, you should use this opportunity to update the password policy for your application to 8 characters.

@driesvints
Copy link
Member Author

ℹ️ If you are using JSON values in your MySQL or MariaDB tables, the query builder now returns unquoted JSON values in Laravel 5.8.

@driesvints
Copy link
Member Author

ℹ️ If you are using SQLite, the oldest supported version in Laravel 5.8 is SQLite 3.7.11. Laravel recommends running SQLite 3.8.8 or higher.

@driesvints
Copy link
Member Author

ℹ️ The Nexmo and Slack notification channels have been extracted into first-party packages. To use these channels in your application, run the following command to require the respective packages:

composer require laravel/nexmo-notification-channel
composer require laravel/slack-notification-channel

@driesvints
Copy link
Member Author

ℹ️ Laravel 5.8 correctly pluralizes multi-word model names ending in a word with an irregular pluralization. For example, Laravel 5.7 incorrectly pluralized the table name for the UserFeedback model as user_feedbacks. Laravel 5.8 correctly pluralizes this table name as user_feedback.

If you were relying on this irregular pluralization, you should rename your table name or set the $table property for your model. If you were overriding this behavior by setting the $table property, you may remove it and now rely on the framework.

@driesvints
Copy link
Member Author

ℹ️ In Laravel 5.8, the env helper treats environment variables as immutable. As such, you may no longer change an environment variable at runtime. Instead, you may use a configuration value which can be retrieved using the config helper.

@joedixon
Copy link
Contributor

❌ Laravel 5.8 has a development dependency for beyondcode/laravel-dump-server of ^1.0. You have a modified dependency for beyondcode/laravel-dump-server in your composer.json. You should review your dependency and ensure it meets the new version requirement.

@driesvints, we have explicit references to versions of beyondcode/laravel-dump-server, filp/whoops and fzaninotto/faker in our composer.json. Are you happy I remove these and utilise the constraints which ship with Laravel or do we need those particular versions?

@driesvints
Copy link
Member Author

@joedixon you can remove dump server and bump the whoops and faker. whoops will go away anyway with the 6.0 shift but you don't need to remove it in this pr.

@joedixon
Copy link
Contributor

joedixon commented Oct 1, 2019

@driesvints I've been though all comments from the shift and updated the relevant parts of the app.

Can you take a look and see if you are happy? I'm assuming you'll want me to squash all the commits of this PR?

Copy link
Member Author

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

I can't request changes as I'm the PR author but I left a bit of more feedback. You can squas once done yeah 👍

Thanks!

@joedixon
Copy link
Contributor

joedixon commented Oct 2, 2019

I made those tweaks @driesvints - let me know if good to go.

@driesvints
Copy link
Member Author

@joedixon looks good. Feel free to squash 👍

@joedixon
Copy link
Contributor

joedixon commented Oct 3, 2019

@driesvints this is good to go

@driesvints driesvints merged commit 5f45efd into master Oct 3, 2019
@driesvints driesvints deleted the shift-19040 branch October 3, 2019 19:23
@driesvints
Copy link
Member Author

I'll buy the 6.0 shift tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants