Paths

Table of Contentst

Differential D25807

Clarify comment in PhutilJSON about required PHP version
ClosedPublic
Actions

Authored by aklapper on Mon, Aug 26, 10:29.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Commits

rARC9f66bff5f663: Clarify comment in PhutilJSON about required PHP version

Summary

Future developers may want to clean up some code after bumping required versions, so explicitly state that JSON_UNESCAPED_SLASHES was introduced in PHP 5.4.0 and that the PHP JSON extension is a core PHP extension since PHP 8.0.0 and cannot be disabled anymore, to save time looking up stuff.

https://www.php.net/ChangeLog-5.php#5.4.0
https://www.php.net/manual/en/json.installation.php

Test Plan

Read the docs.

Diff Detail

Repository

rARC Arcanist

Branch

jsonUnescapedSlashesComment

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 1567
Build 1567: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	769 ms	ArcanistBundleTestCase::testGitRepository EXCEPTION (Exception): Expected patch and actual patch for 5dec8bf28557f078d1987c4e8cfb53d08310f522 differ. Wrote actual patch to '/var/www/html/phorge/arcanist/src/parser/__tests__/patches//5dec8bf28557f078d1987c4e8cfb53d08310f522.gitpatch.real'. #0 /var/www/html/phorge/arcanist/src/parser/__tests__/ArcanistBundleTestCase.php(85): ArcanistBundleTestCase->runGitRepositoryTests(Object(PhutilDirectoryFixture)) #1 /var/www/html/phorge/arcanist/src/unit/engine/phutil/PhutilTestCase.php(639): ArcanistBundleTestCase->testGitRepository()
	42 ms	ArcanistBaseCommitParserTestCase::testBasics 2 assertion(s) passed.
	55 ms	ArcanistBaseCommitParserTestCase::testHalt 1 assertion(s) passed.
	41 ms	ArcanistBaseCommitParserTestCase::testJump 1 assertion(s) passed.
	44 ms	ArcanistBaseCommitParserTestCase::testJumpReturn 1 assertion(s) passed.
		View Full Test Results (1 Failed · 74 Passed)

Event Timeline

aklapper created this revision.Mon, Aug 26, 10:29

Owners added a reviewer: O1: Blessed Committers.Mon, Aug 26, 10:29

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptMon, Aug 26, 10:29

aklapper requested review of this revision.Mon, Aug 26, 10:29

Harbormaster completed remote builds in B1567: Diff 2426.Mon, Aug 26, 10:29

yesyes

This revision is now accepted and ready to land.Mon, Aug 26, 13:45

P.S. try applying D25809 to check if the red unit test becomes green

This revision was landed with ongoing or failed builds.Mon, Aug 26, 14:36

Closed by commit rARC9f66bff5f663: Clarify comment in PhutilJSON about required PHP version. · Explain Why

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rARC9f66bff5f663: Clarify comment in PhutilJSON about required PHP version.

aklapper mentioned this in D25809: ArcanistBundleTestCase: fix support of non-English environments.Mon, Aug 26, 21:41

In D25807#21516, @valerio.bozzolan wrote:

P.S. try applying D25809 to check if the red unit test becomes green

@valerio.bozzolan: It won't - my locale has always been en_US.utf8 so "my" issue isn't localization related (and it seems not to allow me to test your patch, as it "covers" your issue)...

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

parser/

PhutilJSON.php

3 lines

74%

O1: Blessed Committers

Diff 2426

View Options

Clarify comment in PhutilJSON about required PHP versionClosedPublicActions