Page MenuHomeDevCentral

Update Laravel 8
ClosedPublic

Authored by DorianWinty on Apr 16 2022, 22:51.
Tags
  • Restricted Project
Referenced Files
F2748912: D2674.id6766.largetrue.diff
Fri, Mar 29, 11:15
F2748846: D2674.id6763.largetrue.diff
Fri, Mar 29, 10:57
F2747474: D2674.id6796.diff
Fri, Mar 29, 05:40
F2747027: D2674.id6785.diff
Fri, Mar 29, 04:08
F2747019: D2674.id6759.diff
Fri, Mar 29, 04:07
F2747018: D2674.id6768.diff
Fri, Mar 29, 04:07
F2746539: D2674.id6800.largetrue.diff
Fri, Mar 29, 03:04
F2744684: D2674.id6806.largetrue.diff
Thu, Mar 28, 19:08
Subscribers
Tokens
"Yellow Medal" token, awarded by dereckson.

Details

Summary

Update to Laravel 8

Ref T1725

Diff Detail

Repository
rNOTIF Notifications center
Lint
Lint Passed
Unit
Test Failures
Branch
arcpatch-D2674
Build Status
Buildable 4198
Build 4450: arc lint + arc unit

Unit TestsFailed

TimeTest
18 msNasqueron\Notifications\Tests\Actions\ActionsReportTest
Nasqueron\Notifications\Tests\Actions\ActionsReportTest::testReport data/report.json and rendered report differ too much. Try $this->assertEquals($expectedReport, $actualReport) to see a diff.
81 msNasqueron\Notifications\Tests\Config\Reporting\IntegrationTest
Nasqueron\Notifications\Tests\Config\Reporting\IntegrationTest::testConfig ErrorException: Undefined property: Illuminate\Http\JsonResponse::$response
123 msNasqueron\Notifications\Tests\Http\Controllers\GitHubGateControllerTest
Nasqueron\Notifications\Tests\Http\Controllers\GitHubGateControllerTest::testGet Mockery\Exception\BadMethodCallException: Received Mockery_0_Illuminate_Contracts_Events_Dispatcher::dispatch(), but no expectations were specified
118 msNasqueron\Notifications\Tests\Jobs\NotifyNewCommitsToDiffusionTest
130 msNasqueron\Notifications\Tests\PayloadFullTest
Nasqueron\Notifications\Tests\PayloadFullTest::testPost Error: Call to undefined method Nasqueron\Notifications\Tests\PayloadFullTest::seeJson()
View Full Test Results (1 Failed · 6 Broken · 39 Passed)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

adding codestyle
adding on the up of file the class that is use
adding using str_ instead of str::

dereckson added inline comments.
app/Actions/AMQPAction.php
34 ↗(On Diff #6766)

Not in the scope of the upgrade. Those esthetic changes are welcome, but:

  • it's the other way around, ie the PREVIOUS public function __construct ) was the correct one
  • it should be done in a separate change against main branch (we can merge esthetic changes before upgrade without any issue)
app/Actions/Action.php
20 ↗(On Diff #6766)

Probably created by a phpcbf with MediaWiki setting.

The previous code was correct.

33 ↗(On Diff #6766)

Same issue, previous file was fully correct in style.

correct tests
update some packages

correct to make work the tests

DorianWinty retitled this revision from WIP Update Laravel 8 to Update Laravel 8.

modify test
remove unused use

adding back one removed file

dereckson requested changes to this revision.May 8 2022, 17:33
dereckson added inline comments.
app/Analyzers/GitHub/Events/Event.php
34

Extraneous added line

44

extraneous added line

app/Analyzers/GitHub/Events/IssueCommentEvent.php
20

MediaWiki style

app/Analyzers/GitHub/Events/PullRequestEvent.php
18

MediaWiki

tests/TestCase.php
5

Restore that division

  1. Our app
  2. External libs
  3. Global space
This revision now requires changes to proceed.May 8 2022, 17:33
DorianWinty retitled this revision from Update Laravel 8 to WIP Update Laravel 8.May 9 2022, 18:14
DorianWinty marked 5 inline comments as done.

modify folowing comments but need more to reading

DorianWinty retitled this revision from WIP Update Laravel 8 to Update Laravel 8.May 9 2022, 18:31

correcting style problem due to mediawiki phpcs style

correcting MW style to old style

dereckson requested changes to this revision.May 12 2022, 11:37
dereckson added inline comments.
app/Config/Reporting/BaseReportEntry.php
11 ↗(On Diff #6802)

"public abstract" is more convenient as it's coherent with "public static":

That would mean an order <visibility> [special status] function <function name>.

More generally, run automated tools and accept ANY modification they do is a probably not a good idea, git add -p to filter the useful ones is very convenient with those "magic" tools.

app/Providers/RouteServiceProvider.php
36 ↗(On Diff #6802)

MediaWiki style

composer.json
23–24

Two needs to be restored now we're up to date:

  • phploc
  • phpcpd
50

For phpmd, it's well ruleset.xml

tests/Analyzers/GitHub/GitHubPayloadAnalyzerTest.php
68

MediaWiki style

tests/Analyzers/Phabricator/PhabricatorPayloadAnalyzerTest.php
90

MW

tests/Config/FeaturesTest.php
25 ↗(On Diff #6802)

MW

tests/Console/Commands/NotificationsPayloadTest.php
51

MW (the two lines)

tests/Notifications/DockerHubNotificationTest.php
12 ↗(On Diff #6802)

Extraneous line

This revision now requires changes to proceed.May 12 2022, 11:37
DorianWinty marked 9 inline comments as done.

correction with the comments

This revision is now accepted and ready to land.May 21 2022, 19:25
This revision was landed with ongoing or failed builds.May 21 2022, 20:02
This revision was automatically updated to reflect the committed changes.