Page MenuHomeDevCentral

Update Laravel 8
ClosedPublic

Authored by DorianWinty on Apr 16 2022, 22:51.

Details

Summary

Update to Laravel 8

Ref T1725

Diff Detail

Repository
rNOTIF Notifications center
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.Sun, May 8, 17:33
dereckson added inline comments.
app/Analyzers/GitHub/Events/Event.php
34

Extraneous added line

48

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.Sun, May 8, 17:33
DorianWinty retitled this revision from Update Laravel 8 to WIP Update Laravel 8.Mon, May 9, 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.Mon, May 9, 18:31

correcting style problem due to mediawiki phpcs style

correcting MW style to old style

dereckson requested changes to this revision.Thu, May 12, 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
37

MediaWiki style

composer.json
23–24

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

  • phploc
  • phpcpd
55

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

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.Thu, May 12, 11:37
DorianWinty marked 9 inline comments as done.

correction with the comments

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