Upgrade to Symfony 7 #518
Upgrade to Symfony 7 #518
Conversation
|
Another fix is needed to the ToolforgeBundle before this can be morged: wikimedia/ToolforgeBundle#71 |
|
Okay, this is ready for review now, if anyone's got time. |
| @@ -24,12 +24,13 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ ubuntu-latest ] | |||
| php: [ '7.3', '7.4', '8.0', '8.1', '8.2' ] | |||
| # Test against the lowest and highest PHP versions that we support. | |||
| php: [ '8.2', '8.3' ] | |||
There was a problem hiding this comment.
Yeah it kept failing (and fixing it would make it fail for 8.2), and as we're not going to be deploying to PHP 8.4 any time soon I figured it could wait.
src/Util/Semaphore/UnixSemaphore.php
Outdated
| @@ -13,7 +14,7 @@ class UnixSemaphore implements Semaphore { | |||
| private $semaphoreKey; | |||
| /** @var int */ | |||
| private $capacity; | |||
| /** @var resource|null the lazily initialized semaphore descriptor */ | |||
| /** @var resource|SysvSemaphore|null the lazily initialized semaphore descriptor */ | |||
There was a problem hiding this comment.
genuine question: do we still need resource now that we have SysvSemaphore?
(same in other places)
There was a problem hiding this comment.
Good point, no it seems not. Removed!
| */ | ||
| public function testAddAndRetrieve() { | ||
| // Make sure it's empty to start with. | ||
| static::assertEmpty( | ||
| static::assertSame( |
There was a problem hiding this comment.
for me: Is there a difference between static::assertSame and $this->assertSame? I would guess they are the same function and we should harmonize on something?
There was a problem hiding this comment.
Good point. The docs say that there's no right way, and as we've got more $this-> than static I'll stick to that! And also use assertSame over assertEquals unless we want loose checking (which it doesn't look like we do anywhere).
Upgrade to Symfony 7 and update (almost) all PHP dependencies, making as few changes as possible.
Bug: T384115