Paths

Table of Contentst

Differential D25339

Improve arc-browse for php 8.1
ClosedPublic
Actions

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

Tags

None

Referenced Files

	Unknown Object (File)
	Sat, Apr 27, 15:15

	Unknown Object (File)
	Sat, Apr 27, 09:28

	Unknown Object (File)
	Sat, Apr 27, 09:28

	Unknown Object (File)
	Sat, Apr 27, 06:35

	Unknown Object (File)
	Apr 1 2024, 07:32

	Unknown Object (File)
	Apr 1 2024, 07:31

	Unknown Object (File)
	Apr 1 2024, 02:21

	Unknown Object (File)
	Mar 31 2024, 02:04

Subscribers

valerio.bozzolan

Details

Reviewers

valerio.bozzolan
Sten

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15064: Make Phorge compatible with PHP 8.1/8.2/8.3

Commits

rARC6e4947b55f09: Improve arc-browse for php 8.1

Summary

Test Plan

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

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

avivey created this revision.Jul 7 2023, 08:27

Owners added a reviewer: O1: Blessed Committers.Jul 7 2023, 08:27

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJul 7 2023, 08:27

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

Harbormaster completed remote builds in B654: Diff 1102.Jul 7 2023, 08:27

use ===

Harbormaster completed remote builds in B655: Diff 1103.Jul 7 2023, 08:28

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

valerio.bozzolan added inline comments.Jul 7 2023, 08:45

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!

valerio.bozzolan added inline comments.Jul 7 2023, 08:49

src/ref/ArcanistRepositoryRef.php
95–96	By the way I can confirm this bug in Subversion ↑

coalesce

Harbormaster completed remote builds in B656: Diff 1104.Jul 7 2023, 09:02

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?

valerio.bozzolan added inline comments.Jul 7 2023, 11:51

src/ref/ArcanistRepositoryRef.php
95–96	Yep. T15541

Sten accepted this revision.Jul 7 2023, 13:32

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

Closed by commit rARC6e4947b55f09: Improve arc-browse for php 8.1. · Explain WhyJul 7 2023, 16:17

This revision was automatically updated to reflect the committed changes.

avivey added a commit: rARC6e4947b55f09: Improve arc-browse for php 8.1.

Revision Contents
Changeset List

Path

Size

Packages

src/

browse/

query/

ArcanistBrowseObjectNameURIHardpointQuery.php

4 lines

O1: Blessed Committers

workflow/

ArcanistBrowseWorkflow.php

4 lines

O1: Blessed Committers

ref/

ArcanistRepositoryRef.php

2 lines

O1: Blessed Committers

Diff 1106

src/browse/query/ArcanistBrowseObjectNameURIHardpointQuery.php

Loading...

src/browse/workflow/ArcanistBrowseWorkflow.php

Loading...

src/ref/ArcanistRepositoryRef.php

Loading...