Make everything 1000x or 1000000x slower
ClosedPublic
Actions

Authored by epriestley on May 22 2015, 8:19 PM.

Details

Reviewers

joshuaspence
btrahan

Commits

Restricted Diffusion Commit
rP7d757483a0db: Make everything 1000x or 1000000x slower

Summary

In the great pht() conversion, some strings like "123,456" are now being printed as numbers with "%d". These come out as "123" instead of "123,456".

Use "%s" and "PhutilNumber" to present numbers with comma groupings.

Test Plan

Viewed DarkConsole.
Viewed conduit logs.
Viewed daemon logs.
Grepped for %d ms and %d us.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 31284.May 22 2015, 8:19 PM

epriestley retitled this revision from to Make everything 1000x or 1000000x slower.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added reviewers: btrahan, joshuaspence.

Herald added a subscriber: epriestley. · View Herald TranscriptMay 22 2015, 8:19 PM

Would it be unreasonable to have %d always use PhutilNumber?

This revision is now accepted and ready to land.May 22 2015, 11:29 PM

Constructs like pht('T%d: %s', $task_id, $task_name) or should use integers since T1,234 isn't meaningful.

Sure, but how common is that? It could be written as pht('%s: %s', "T{$task_I'd}", $task_name) instead.

It's probably less common than it once was since we started introducing getMonogram(), but probably would need a migration pass if we were to change it.

We also don't actually parse the %d / %s stuff, and doing so might be unacceptably slow. We could statically forbid %d and then (possibly) auto-box any integers that were passed in with a PhutilNumber. This might be an improvement overall, given that getMonogram() should handle the most common use of %d and you could still force your way around it with (string)$id or whatever if you really didn't want commas.