joshuaspence (Joshua Spence)
Code Monkey

Projects (80)

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Sunday

  • Clear sailing ahead.

User Details

User Since
Oct 9 2013, 12:25 AM (193 w, 2 d)
Availability
Available

Recent Activity

Wed, May 31

joshuaspence awarded D18053: Help PROFESSIONAL SOFTWARE ENGINEERS copy text to their clipboard a Mountain of Wealth token.
Wed, May 31, 9:14 PM
joshuaspence added a comment to T1026: Request: ability to select line range for comment via touchscreen.

I currently expect to revert this change and decline to implement the feature because it makes using Differential on a phone more difficult for left-handed users

Wed, May 31, 7:05 AM · Inline Comments, Differential

Tue, May 30

joshuaspence added a comment to Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery` (Answer 548).

Thanks for the very detailed response.

Tue, May 30, 1:52 AM

Mon, May 29

joshuaspence abandoned D18040: Maintain POST requests when following redirects.

Yep, that's a better solution. In the mean time, we just updated all of our .arcconfig files.

Mon, May 29, 11:31 PM

Sun, May 28

joshuaspence added a comment to D18040: Maintain POST requests when following redirects.

@epriestley, if you had any thoughts on this it would be appreciated as this issue is heavily affecting our users. There is a workaround (update our .arcconfig files to point to the new domain), but we have a lot of repositories...

Sun, May 28, 11:04 PM
joshuaspence created D18040: Maintain POST requests when following redirects.
Sun, May 28, 11:03 PM
joshuaspence created T12769: Applications page shows Diffusion on admin.
Sun, May 28, 2:26 AM · Phacility, Bug Report

Thu, May 25

joshuaspence added a comment to Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery` (Answer 548).

For git, is that because discovery only happens on the Almanac device on which the repository is bound to or is it because there is some sort of global lock taken for repository updates? I'm curious to learn more bout how this works.

Thu, May 25, 8:51 PM
joshuaspence marked Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery` (Answer 549) as hidden.
Thu, May 25, 8:51 PM
joshuaspence updated the answer details for Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery` (Answer 549).
Thu, May 25, 8:50 PM
joshuaspence added a comment to Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery` (Answer 548).

Thanks.

Thu, May 25, 8:41 PM
joshuaspence created Q622: It is still a requirement that only one repo pull daemon runs without `--no-discovery`.
Thu, May 25, 8:23 PM · Clusters
joshuaspence closed T12754: 404 when trying to create a new repository as Invalid.

This was a misconfiguration on my part and I wasn't properly forwarding the query string from nginx to PHP.

Thu, May 25, 1:17 AM · Bug Report
joshuaspence closed T12756: FontAwesome is returning a 404 as Invalid.

This was a misconfiguration on my part and I wasn't properly forwarding the query string from nginx to PHP.

Thu, May 25, 1:17 AM · Bug Report
joshuaspence created T12756: FontAwesome is returning a 404.
Thu, May 25, 1:10 AM · Bug Report
joshuaspence updated the task description for T12755: Aphlict doesn't work with the latest version of `ws`.
Thu, May 25, 12:46 AM · Aphlict, Bug Report
joshuaspence created T12755: Aphlict doesn't work with the latest version of `ws`.
Thu, May 25, 12:45 AM · Aphlict, Bug Report

Wed, May 24

joshuaspence added a comment to T12754: 404 when trying to create a new repository.

Possibly related, I'm having issues with Almanac too. I created an Almanac device, but clicking on "Add Interface" takes me to /almanac/interface/edit/?deviceID=2 which shows a 404.

Wed, May 24, 9:55 PM · Bug Report
joshuaspence created T12754: 404 when trying to create a new repository.
Wed, May 24, 9:54 PM · Bug Report
joshuaspence added a comment to T12547: Confusing error message when trying to register an account over HTTP with `security.require-https`.

Was this on a test install on localhost? I can only reproduce this if the remote address is part of cluster.addresses.

Wed, May 24, 9:43 PM · Auth, Bug Report
tomekj2ee awarded T12144: Ability to reorder milestones on a project's workboard a Like token.
Wed, May 24, 6:37 PM · Projects, Feature Request

May 23 2017

joshuaspence added a comment to T12720: Record of hibernating daemon was garbage collected.

Ah this probably explains what I have observed on our installation too.

May 23 2017, 2:27 PM · Customer Impact, Daemons

May 16 2017

joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

As in... If users hot this error from non-CLI mode then they probably need to change the file permissions or add their web user to a group. If users hit this from the CLI then they need sudo

May 16 2017, 10:08 PM
joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

We could tailor the message differently based on CLI/FPM?

May 16 2017, 10:07 PM
joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

Do you have some text which you think would be more clear?

May 16 2017, 10:03 PM
joshuaspence removed 1 blocking reviewer(s) for D17917: Throw an exception if `local.json` can't be read: Blessed Reviewers.
May 16 2017, 10:01 PM
joshuaspence accepted D17917: Throw an exception if `local.json` can't be read.
May 16 2017, 10:01 PM
joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

It may have permissions which are too restrictive.

May 16 2017, 10:00 PM
joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

Sure.

May 16 2017, 9:53 PM
joshuaspence updated the test plan for D17917: Throw an exception if `local.json` can't be read.
May 16 2017, 9:51 PM
joshuaspence updated the diff for D17917: Throw an exception if `local.json` can't be read.

Use Filesystem class

May 16 2017, 9:51 PM
joshuaspence added a comment to D17917: Throw an exception if `local.json` can't be read.

I'm happy to bring a version of this upstream, but it should probably Filesystem::readFile(...), phutil_json_decode(), and distinguish between "can not read" and "read it fine, but your JSON is junk".

May 16 2017, 9:45 PM
joshuaspence created D17917: Throw an exception if `local.json` can't be read.
May 16 2017, 9:38 PM
joshuaspence added a comment to T12607: Decrease pain and suffering caused by deploy/upgrade process.

A vaguely related question... how is the /status/ endpoint meant to work behind an ELB? AWS ELB's don't pass a Host header to the healthcheck endpoint, so by default it just returns a 500. I worked around this by using some nginx voodoo to rewrite the Host header if the User-Agent header matches ^ELB-HealthChecker/\d+\.\d+$, but this is less-than-ideal.

May 16 2017, 12:43 PM · Phacility

May 15 2017

joshuaspence created Q617: Does `repository.default-local-path` need to exist if I am not storing repositories on the web server?.
May 15 2017, 9:34 PM · Setup

May 13 2017

joshuaspence added a comment to T12706: Add feature to "diff against" comments in Differential.

I think the author is essentially asking for an easy way to map inline comments to the diff that they were originally made upon.

May 13 2017, 3:52 AM · Feature Request
joshuaspence added a comment to T5155: Evaluate support for AWS IAM Roles in S3 Client.

As a general note here, we've been vulnerable to credential theft from the local service throughout this discussion, and still are until T12701 resolves: attackers can create a Harbormaster build plan which sends requests to 169.254.169.254, then read credentials from the output of the "failed build".

May 13 2017, 12:44 AM · Files

May 11 2017

joshuaspence awarded T12664: Update diff/patch parsing to extract more metadata and parse a wider range of formats a Pirate Logo token.
May 11 2017, 12:29 PM · Arcanist, Differential

May 9 2017

joshuaspence created D17854: Fix `puppet-lint` linter.
May 9 2017, 2:08 AM

May 8 2017

joshuaspence added a comment to T12689: Mail is still received after resigning from a revision.

(But what if you meant "I'm not going to review this but still want to keep an eye on it" instead of "I'm not going to review this and don't care about it at all"?)

May 8 2017, 11:58 PM · Mail, Differential, Bug Report
joshuaspence added a comment to T12689: Mail is still received after resigning from a revision.

rPbcd87e0e3f3756970d26d5e9c4a60e4be73ef6a6
rARC3c4735795a2963c5ddff6dceaf60122d01ca3dc0
rPHUa900d7b63e954e221efe140f0f33d3d701524aae

May 8 2017, 10:37 PM · Mail, Differential, Bug Report
joshuaspence created T12689: Mail is still received after resigning from a revision.
May 8 2017, 10:21 PM · Mail, Differential, Bug Report
joshuaspence added a comment to D17839: Always show user data from test results.

Another possibility here would to be introduce ArcanistUnitTestResult::RESULT_UNSTABLE.

May 8 2017, 2:34 AM
joshuaspence created D17839: Always show user data from test results.
May 8 2017, 2:24 AM

May 4 2017

joshuaspence accepted D17783: Improve `ArcanistLinterStandardTestCase`.

LGTM. @epriestley, it would be greatly appreciated if this could be accepted upstream. We have a large number of XHPAST linter rules (I think ~200) and we want to write integration tests to ensure that they play nicely together.

May 4 2017, 12:02 AM

May 3 2017

joshuaspence accepted D17820: Replace all the PHPAST JSON test data with readable test data.

I'm just going to trust that you will fix this after D17819 lands.

May 3 2017, 11:49 PM
joshuaspence added a comment to D17820: Replace all the PHPAST JSON test data with readable test data.

I think this needs to be updated? It fails for me locally.

May 3 2017, 11:34 PM
joshuaspence accepted D17819: Make PHPAST parser tests stable and human-readable.
May 3 2017, 11:29 PM
joshuaspence updated subscribers of D17819: Make PHPAST parser tests stable and human-readable.

Just CCing some people that I work with that will be interested in this change.

May 3 2017, 10:28 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

Yeah sorry, I meant addcslashes with quotes. I agree with you regarding readability, but I think developers are used to quotes and escacaping anyway.

May 3 2017, 10:14 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

That one string won't align properly but output will otherwise be stable. We could make a decision to break existing tests to add more padding if we wanted.

The padding as written felt more readable to me than breaking things onto separate lines, because many tokens are short so this requires about half as many lines, and it makes it easier to scan the token values by reading the right-hand column.

I suppose I can dynamically pad the column to length of the longest string it actually uses to future-proof this.

May 3 2017, 10:10 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

I just think it would be better to use real escaping, rather than people needing to know that we change spaces to underscores. Can we use addcslashes perhaps? Sorry if I am nitpicking here, feel free to just ignore me.

May 3 2017, 10:08 PM
joshuaspence added inline comments to D17819: Make PHPAST parser tests stable and human-readable.
May 3 2017, 10:01 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

The wrapping is just to make things more readable and diff-able for tests which include, e.g., large HEREDOCs, nodes with long token sequences, etc., which may span multiple lines. Letting the lines go out to a billion characters would be the same from the perspective of test correctness, but presumably less readable for humans, and 70,000 character lines or whatever tend to be a bit of a mess in Differential.

May 3 2017, 9:58 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

I can't really see us getting much use out of it. I also didn't actually update it here or in the next change because the test continues to pass when the output changes.

May 3 2017, 9:57 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

(If we don't change spaces to underscores we get a bunch of result-affecting trailing whitespace which my editor strips and which is invisible in diffs.)

May 3 2017, 9:56 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

(If we don't change spaces to underscores we get a bunch of result-affecting trailing whitespace which my editor strips and which is invisible in diffs.)

May 3 2017, 9:54 PM
joshuaspence added inline comments to D17819: Make PHPAST parser tests stable and human-readable.
May 3 2017, 9:53 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

Just submitting some comments whilst I go along, in case I don't get a chance to fully review this.

May 3 2017, 9:51 PM
joshuaspence added inline comments to D17819: Make PHPAST parser tests stable and human-readable.
May 3 2017, 9:43 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

If you want to test the test logic, just write a normal test aimed at whatever feature you want to test (e.g., encoding of multiline tokens or utf8 strings or whatever)? You're testing a little more than purely the wrap/encode functions, but since the input can be very small I think you'll almost exclusively hit the code you actually care about.

The formatting overall feels very unit-test specific to me and I'd worry that putting it in XHPASTTree could encourage off-label use and end in teras. Like, we'd end up with getReadableTreeUseThisMethodONLYForUnitTests_OR_ELSE().

May 3 2017, 9:38 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

In D17817, you added a new token in the middle, so Flex/Bison learned about it before some other token, assigned it the next number, and threw everything after it out of whack. That's basically fine -- there's no real value in keeping the constants stable, as long as we don't encode the constant values in unit tests.

May 3 2017, 9:36 PM
joshuaspence added inline comments to D17819: Make PHPAST parser tests stable and human-readable.
May 3 2017, 9:17 PM
joshuaspence added a comment to D17819: Make PHPAST parser tests stable and human-readable.

The actual numeric constants aren't stable across PHP versions, so these tests can fail for literally no reason.

May 3 2017, 9:10 PM
joshuaspence added a comment to D17817: Add support for exponentiation.

I definitely did something wrong here, because I updated src/parser/xhpast/parser_tokens.php by hand...

May 3 2017, 12:00 PM
joshuaspence created D17817: Add support for exponentiation.
May 3 2017, 12:00 PM
joshuaspence added a revision to T4334: Support PHP5.4+ syntax in XHPAST: D17817: Add support for exponentiation.
May 3 2017, 12:00 PM · XHPAST
joshuaspence created D17816: Add support for constant scalar expressions.
May 3 2017, 11:42 AM
joshuaspence added a revision to T4334: Support PHP5.4+ syntax in XHPAST: D17816: Add support for constant scalar expressions.
May 3 2017, 11:42 AM · XHPAST

Apr 21 2017

joshuaspence added a comment to D17756: Remove all "Phabricator Bot" code.

We use the bot quite heavily at my company. Is there anything stopping me from just copying the deleted code into a libphutil extension and continuing to use it? Really, PhabricatorBotLogHandler is what we care about most.

Apr 21 2017, 9:57 PM

Apr 17 2017

joshuaspence updated the task description for T12578: Herald rule for "Differential reviewers do not include" didn't fire when I expected it to.
Apr 17 2017, 11:15 PM · Differential, Herald, Bug Report
joshuaspence added a comment to T12578: Herald rule for "Differential reviewers do not include" didn't fire when I expected it to.

Hmm ok. I think that I'm not usually listed explicitly as a reviewer, which means that Reviewers: #some_project_that_i_am_a_member_of doesn't include me. Would you be in favor of adding a new [Differential was reviewed by] field to Herald?

Apr 17 2017, 10:56 PM · Differential, Herald, Bug Report
joshuaspence created T12578: Herald rule for "Differential reviewers do not include" didn't fire when I expected it to.
Apr 17 2017, 10:48 PM · Differential, Herald, Bug Report

Apr 13 2017

joshuaspence added inline comments to D17684: Add validation for config settings of type regex.
Apr 13 2017, 11:31 PM
joshuaspence added inline comments to D17684: Add validation for config settings of type regex.
Apr 13 2017, 11:30 PM
joshuaspence created T12548: Unable to initialize database with invalid credentials in configuration file.
Apr 13 2017, 7:01 AM · Database, Bug Report
joshuaspence renamed T12547: Confusing error message when trying to register an account over HTTP with `security.require-https` from Unable to create initial user account to Confusing error message when trying to register an account over HTTP with `security.require-https`.
Apr 13 2017, 6:21 AM · Auth, Bug Report
joshuaspence added a comment to T12547: Confusing error message when trying to register an account over HTTP with `security.require-https`.

Actually, I figured it out... The problem was that I had security.require-https set to true but I hadn't yet setup HTTPS so I was only using plain old HTTP.

Apr 13 2017, 6:20 AM · Auth, Bug Report
joshuaspence added a comment to T12547: Confusing error message when trying to register an account over HTTP with `security.require-https`.

I tried to reproduce this on a Phacility host, but it doesn't reproduce there because the first user account is setup via OAuth.

Apr 13 2017, 5:50 AM · Auth, Bug Report
joshuaspence created T12547: Confusing error message when trying to register an account over HTTP with `security.require-https`.
Apr 13 2017, 5:49 AM · Auth, Bug Report

Apr 11 2017

joshuaspence added a comment to T12534: Differential still appears in "responsible users" query after resigning as a reviewer.

When I turn on bucketing, I can't see it actually. Although I do have a lot of diffs, so it could have overheated.

Apr 11 2017, 7:22 AM · Bug Report (Needs Information), Differential
joshuaspence added a comment to T12534: Differential still appears in "responsible users" query after resigning as a reviewer.
Apr 11 2017, 6:39 AM · Bug Report (Needs Information), Differential
joshuaspence created T12534: Differential still appears in "responsible users" query after resigning as a reviewer.
Apr 11 2017, 5:27 AM · Bug Report (Needs Information), Differential
joshuaspence added a comment to D17651: Correct two parameter strictness issues with file uploads.

How could you??

Apr 11 2017, 12:13 AM

Apr 10 2017

joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

This seems to have fixed the issue. Thanks for the quick fix.

Apr 10 2017, 11:07 PM · Arcanist, Files, Bug Report
epriestley awarded T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0 a Mountain of Wealth token.
Apr 10 2017, 11:03 PM · Arcanist, Files, Bug Report
joshuaspence accepted D17651: Correct two parameter strictness issues with file uploads.
Apr 10 2017, 11:00 PM
joshuaspence added inline comments to D17651: Correct two parameter strictness issues with file uploads.
Apr 10 2017, 10:58 PM
joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

Not sure if this is useful:

Apr 10 2017, 10:52 PM · Arcanist, Files, Bug Report
joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

Hmm, I got a different error with a 9MB file. But this looks like a problem on our end:

Apr 10 2017, 10:39 PM · Arcanist, Files, Bug Report
joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

I originally tried a 1GB file and wasn't able to reproduce...

Apr 10 2017, 10:38 PM · Arcanist, Files, Bug Report
joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

From the server-side error logs:

Apr 10 2017, 10:35 PM · Arcanist, Files, Bug Report
joshuaspence added a project to T12532: `syntax.filemap` does not validate input: Config.
Apr 10 2017, 10:33 PM · Contributor Onboarding, Config, Bug Report
joshuaspence created T12532: `syntax.filemap` does not validate input.
Apr 10 2017, 10:33 PM · Contributor Onboarding, Config, Bug Report
joshuaspence added a comment to T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.

Okay, I managed to reproduce it with non-sensitive data:

Apr 10 2017, 10:28 PM · Arcanist, Files, Bug Report
joshuaspence created T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0.
Apr 10 2017, 10:21 PM · Arcanist, Files, Bug Report

Apr 6 2017

joshuaspence created T12514: `refs/keep-around/` commits should be ignored.
Apr 6 2017, 10:17 PM · Diffusion, Bug Report

Mar 28 2017

joshuaspence accepted rP8879118b696f: Fix a mid-air collision around SearchService roles.
Mar 28 2017, 9:19 PM

Mar 21 2017

joshuaspence added inline comments to D17497: Use "git ls-remote" to guess if "git fetch" is a no-op.
Mar 21 2017, 1:41 AM

Mar 20 2017

joshuaspence added a comment to D17232: Improve `ArcanistDeprecationXHPASTLinterRule` by lowercasing keys in `xhpast.deprecated.functions`.

LGTM, but we need a Blessed Reviewers to sign off on it as well.

Mar 20 2017, 7:32 AM