Page MenuHomePhorge

Improve arc-browse for php 8.1
ClosedPublic

Authored by avivey on Jul 7 2023, 08:27.

Details

Summary

Ref T15064.

Test Plan

ran some arc browse commands in php 8.1, got no exceptions.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

avivey requested review of this revision.Jul 7 2023, 08:27

Thaanks

Tested with a directory, a file, a sub-file, a sub-directory. It still works ✨

This revision is now accepted and ready to land.Jul 7 2023, 08:36
Sten requested changes to this revision.Jul 7 2023, 08:40
Sten subscribed.
Sten added inline comments.
src/ref/ArcanistRepositoryRef.php
82–84

Simply checking for null and then letting the null continue on down isn't the best solution here, as it is subsequently put into the return string.
Better to guarantee $uri_base is a string using coalesce().

This revision now requires changes to proceed.Jul 7 2023, 08:40
src/ref/ArcanistRepositoryRef.php
82–84

I agree premising that the only use was "{$uri_base}" and it does not cause problems since (string) null is always casted to an empty string as expected. Just to say: it was weird (because PHP is weird) but under control.

avivey planned changes to this revision.Jul 7 2023, 08:46
avivey added inline comments.
src/ref/ArcanistRepositoryRef.php
82–84

nice, you're right!

src/ref/ArcanistRepositoryRef.php
95–96

By the way I can confirm this bug in Subversion ↑

avivey marked an inline comment as done.Jul 7 2023, 09:02
avivey added inline comments.
src/ref/ArcanistRepositoryRef.php
95–96

have SVN repository and run arc browse?

src/ref/ArcanistRepositoryRef.php
95–96

Yep. T15541

This revision is now accepted and ready to land.Jul 7 2023, 13:32
valerio.bozzolan retitled this revision from Improve arc-browse for php 8 to Improve arc-browse for php 8.1.Jul 7 2023, 14:35
This revision was automatically updated to reflect the committed changes.