Page MenuHomeDevCentral

Update Laravel 8
ClosedPublic

Authored by DorianWinty on Apr 16 2022, 22:51.
Tags
  • Restricted Project
Referenced Files
F2743911: D2674.id6781.largetrue.diff
Thu, Mar 28, 12:31
F2743903: D2674.id6800.largetrue.diff
Thu, Mar 28, 12:30
F2743496: D2674.id6799.largetrue.diff
Thu, Mar 28, 10:10
F2743494: D2674.id6807.largetrue.diff
Thu, Mar 28, 10:09
F2743019: D2674.id6768.diff
Thu, Mar 28, 07:22
F2742902: D2674.id6790.largetrue.diff
Thu, Mar 28, 06:57
F2741201: D2674.id6801.largetrue.diff
Wed, Mar 27, 23:41
F2741151: D2674.id6799.largetrue.diff
Wed, Mar 27, 23:27
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 4199
Build 4451: arc lint + arc unit

Unit TestsFailed

TimeTest
24 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.
154 msNasqueron\Notifications\Tests\Analyzers\GitHubPayloadAnalyzerTest
Nasqueron\Notifications\Tests\Analyzers\GitHubPayloadAnalyzerTest::testGetGroupOnPushToMappedRepository Failed asserting that two strings are identical.
106 msNasqueron\Notifications\Tests\Analyzers\ItemGroupMappingTest
Nasqueron\Notifications\Tests\Analyzers\ItemGroupMappingTest::testDoesItemMatch Failed asserting that false is true.
180 msNasqueron\Notifications\Tests\Analyzers\Phabricator\PhabricatorGroupMappingTest
Nasqueron\Notifications\Tests\Analyzers\Phabricator\PhabricatorGroupMappingTest::testDoesProjectBelong Failed asserting that true is false.
94 msNasqueron\Notifications\Tests\Config\Reporting\IntegrationTest
Nasqueron\Notifications\Tests\Config\Reporting\IntegrationTest::testConfig ErrorException: Undefined property: Illuminate\Http\JsonResponse::$response
View Full Test Results (4 Failed · 6 Broken · 55 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
26–27

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
16

Probably created by a phpcbf with MediaWiki setting.

The previous code was correct.

22–23

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
32

Extraneous added line

42

extraneous added line

app/Analyzers/GitHub/Events/IssueCommentEvent.php
11–12

MediaWiki style

app/Analyzers/GitHub/Events/PullRequestEvent.php
11–12

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
6–7

"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
29–30

MediaWiki style

composer.json
23–24

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

  • phploc
  • phpcpd
51

For phpmd, it's well ruleset.xml

tests/Analyzers/GitHub/GitHubPayloadAnalyzerTest.php
10

MediaWiki style

tests/Analyzers/Phabricator/PhabricatorPayloadAnalyzerTest.php
10

MW

tests/Config/FeaturesTest.php
9–10

MW

tests/Console/Commands/NotificationsPayloadTest.php
48

MW (the two lines)

tests/Notifications/DockerHubNotificationTest.php
10

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.