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

Laravel 8 upgrade #285

Merged
merged 24 commits into from
Oct 25, 2020
Merged

Laravel 8 upgrade #285

merged 24 commits into from
Oct 25, 2020

Conversation

supertassu
Copy link
Collaborator

@supertassu supertassu commented Sep 10, 2020 •
edited
Loading

see #103 for more details. this pr also includes #286

  • Update dependencies
  • Use php artisan test instead of vendor/bin/phpunit
  • Update database factories to Laravel 8 format
  • Apply all changes made between 6 and 8 in the application skeleton

@supertassu supertassu self-assigned this Sep 10, 2020
@pull-assistant
Copy link

pull-assistant bot commented Sep 10, 2020 •
edited
Loading

Score: 0.98

Best reviewed: commit by commit

Optimal code review plan (1 warning)

Start updating to Laravel 8

app/Exceptions/Handler.php 89% changes removed in Apply non-model-movi...

     Fix tests, update laravel-socialite-mediawiki

     Only search oldappeals table if it exists in AppealController::appeal(...

     Merge branch 'appeal-oldappeal-check' into laravel-8

     Properly throw exception

     Merge branch 'appeal-oldappeal-check' into laravel-8

     Fix class name in test

     Apply non-model-moving changes made between L6 and 8

     Rename database/seeds/ to database/seeders/

     Manually apply namespace to routes, comments on variable seem wrong

     Update dependencies again

     Class name fixes

Powered by Pull Assistant. Last update 5565b16 ... a796cb1. Read the comment docs.

@supertassu supertassu marked this pull request as ready for review September 11, 2020 18:44
@supertassu supertassu mentioned this pull request Sep 12, 2020
Closed
This was referenced Sep 15, 2020
Closed
Merged
@dqwiki dqwiki linked an issue Sep 15, 2020 that may be closed by this pull request
Update to Laravel 8 #103
Closed
dqwiki
dqwiki reviewed Oct 22, 2020
View reviewed changes
app/Http/Controllers/AppealController.php Outdated
Comment on lines 90 to 92
if (!is_null($comment->commentUser) && !in_array($comment->commentUser, $userlist)) {
$userlist[$comment->commentUser] = Olduser::findOrFail($comment->commentUser)->username;
}
Copy link
Member

Choose a reason for hiding this comment

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

This is not a sufficient catch. With Oldappeals, Oldcomments has a zero equivalency being the system. Uploading a patch.

dqwiki
dqwiki approved these changes Oct 22, 2020
View reviewed changes
Copy link
Member

@dqwiki dqwiki left a comment

Choose a reason for hiding this comment

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

All checks seem to be passing, quick run through on the interface fine. 1 bug found via CR, and fixed.

@dqwiki dqwiki mentioned this pull request Oct 22, 2020
Closed
@supertassu
Copy link
Collaborator Author

Thanks. Feel free to merge, and remember to update dependencies on production when deploying there.

@dqwiki dqwiki merged commit 44664cf into master Oct 25, 2020
@dqwiki dqwiki deleted the laravel-8 branch October 25, 2020 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dqwiki dqwiki dqwiki approved these changes

Assignees

@supertassu supertassu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update to Laravel 8
2 participants
@supertassu @dqwiki