Page MenuHomePhabricator

Firehed (Eric Stern)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Sunday

  • Clear sailing ahead.

User Details

User Since
Jul 25 2013, 6:56 PM (304 w, 15 h)
Availability
Available

Recent Activity

May 3 2017

Firehed awarded D17819: Make PHPAST parser tests stable and human-readable a 100 token.
May 3 2017, 5:16 PM

Feb 24 2017

Firehed added a comment to T11741: Quickstart can fail to initialize databases if MyISAM is not available (currently, only in Google Cloud).

Just a note for the above - the initial DB seed script will need to be updated accordingly as well. Right now, trying to perform a clean install on a 2nd-gen Google Cloud SQL instance (which has the above limitation) simply errors out when it reaches one of the tables coded as MyISAM.

Feb 24 2017, 11:06 PM

Jul 20 2016

sascha-egerer awarded D15880: Move 'should render' logic to PhutilUnitTestEngine a Like token.
Jul 20 2016, 10:55 AM

Jun 14 2016

svemir awarded D15880: Move 'should render' logic to PhutilUnitTestEngine a Like token.
Jun 14 2016, 7:51 PM
svemir awarded D15895: Modernize PhpunitTestEngine to work with .arcunit a Like token.
Jun 14 2016, 7:24 PM

May 12 2016

Firehed added a comment to D15895: Modernize PhpunitTestEngine to work with .arcunit.

Noted. This is by no means an urgent thing (I have a Composer package to mostly the same effect, it just seems silly to be maintaining it twice), and all else being equal I'd rather spend time thinking about the bigger solution. But in the name of all things holy, please don't reinvent Composer as T5055 is basically getting at.

May 12 2016, 7:11 PM
Firehed added a comment to D15895: Modernize PhpunitTestEngine to work with .arcunit.

I'd like to get some feedback on the initial implementation, I'll go back and address the couple of TODOs. Note that the change to PhutilUnitTestEngine and one change in ArcanistConfigurationDrivenUnitEngine are actually D15880 because I branched from the wrong place.

May 12 2016, 1:07 AM
Firehed retitled D15895: Modernize PhpunitTestEngine to work with .arcunit from to Modernize PhpunitTestEngine to work with .arcunit.
May 12 2016, 1:04 AM

May 10 2016

Firehed added a comment to D15880: Move 'should render' logic to PhutilUnitTestEngine.

I'm trying to get the PHPUnit engine working with .arcunit, like the phutil engine. Without this change, the results will never render unless the engine renders its own results (as phutil currently does). The Arc Unit workflow itself skips rendering the overall results since ArcanistConfigurationDrivenTestEngine::shouldEchoTestResults() returns false, as it contains logic to render results (changed in this diff).

May 10 2016, 7:55 PM
Firehed added a comment to D15880: Move 'should render' logic to PhutilUnitTestEngine.

After applying the diff locally, in PhutilUnitTestEngine change shouldEchoTestResults to return true (or comment out the addition entirely, since the parent does the same):

May 10 2016, 7:28 PM
Firehed retitled D15880: Move 'should render' logic to PhutilUnitTestEngine from to Move 'should render' logic to PhutilUnitTestEngine.
May 10 2016, 7:09 PM

Apr 28 2016

Firehed added a comment to D15678: Add basic support for return type hints to xhpast.

(That sounded really self-congraulatory, but it took me a lot of work to come up with the node tree + token list approach!)

Apr 28 2016, 9:04 PM
Firehed added a comment to D15678: Add basic support for return type hints to xhpast.

Related: it may be worth looking into leveraging the native AST going forward instead of trying to keep xhpast in sync with PHP with every new minor release. I'd wager it wouldn't take too much voodoo to convert between them so that the existing linter rules continue to work.

Apr 28 2016, 8:19 PM

Feb 24 2016

Firehed added a comment to D15343: Filter out some Clover coverage to prevent false positives.

Thanks!

Feb 24 2016, 8:59 PM
Firehed committed rARC086f5399bfbb: Filter out some Clover coverage to prevent false positives (authored by Firehed).
Filter out some Clover coverage to prevent false positives
Feb 24 2016, 8:58 PM
Firehed closed D15343: Filter out some Clover coverage to prevent false positives by committing rARC086f5399bfbb: Filter out some Clover coverage to prevent false positives.
Feb 24 2016, 8:58 PM
Firehed closed T10420: Code coverage reports from `arc unit` are sometimes inaccurate as Resolved by committing rARC086f5399bfbb: Filter out some Clover coverage to prevent false positives.
Feb 24 2016, 8:58 PM · Arcanist, Bug Report
Firehed retitled D15343: Filter out some Clover coverage to prevent false positives from to Filter out some Clover coverage to prevent false positives.
Feb 24 2016, 8:55 PM
Firehed added a revision to T10420: Code coverage reports from `arc unit` are sometimes inaccurate: D15343: Filter out some Clover coverage to prevent false positives.
Feb 24 2016, 8:55 PM · Arcanist, Bug Report
Firehed added a comment to T10420: Code coverage reports from `arc unit` are sometimes inaccurate.

Implementation of what's described above - also seems to provide the expected results in my case, and limits impact to PHPUnit results. If a file is executed at all, it should show up normally; when completely untouched (which tends to show up with phpunit whitelisting, now necessary in the newest versions).

Feb 24 2016, 8:40 PM · Arcanist, Bug Report
Firehed added a comment to T10420: Code coverage reports from `arc unit` are sometimes inaccurate.

That seems totally reasonable. It seems like a bit of a stretch for PHP code specifically, but sane as a general-purpose answer.

Feb 24 2016, 5:36 PM · Arcanist, Bug Report

Feb 23 2016

Firehed created T10420: Code coverage reports from `arc unit` are sometimes inaccurate.
Feb 23 2016, 1:48 AM · Arcanist, Bug Report

Feb 6 2015

Firehed retitled D11704: Use --hex-blob flag in bin/storage dump from to Use --hex-blob flag in bin/storage dump.
Feb 6 2015, 8:18 PM

Oct 21 2014

joshuaspence awarded D10728: Allow `arc liberate` to handle traits and namespaces a Mountain of Wealth token.
Oct 21 2014, 9:37 AM

Oct 20 2014

Firehed added inline comments to D10728: Allow `arc liberate` to handle traits and namespaces.
Oct 20 2014, 8:11 PM
Firehed updated D10728: Allow `arc liberate` to handle traits and namespaces.
Oct 20 2014, 8:02 PM
Firehed retitled D10728: Allow `arc liberate` to handle traits and namespaces from to Allow `arc liberate` to handle traits and namespaces.
Oct 20 2014, 8:00 PM

Oct 16 2014

Firehed retitled D10717: Widen scope of ArcanistLintEngine::buildLinters from to Widen scope of ArcanistLintEngine::buildLinters.
Oct 16 2014, 8:13 PM

Jul 31 2014

Firehed added a comment to D9792: Add namespace support in Diviner's PHP atomizer.

Turns out this is actually illegal (as I thought):

Jul 31 2014, 5:51 PM

Jul 25 2014

Firehed added a comment to D9792: Add namespace support in Diviner's PHP atomizer.

This looks great to me, but I think I came up with a "clever" construction it gets wrong.

My word, why would they allow syntax like that? I'll write a test case for it and see what happens, but that's probably just "give up you damn fool" territory :)

Jul 25 2014, 7:53 PM

Jul 14 2014

Firehed accepted D9904: Separate display name/context from normalized name/context in Diviner.

This looks good to me - I'll update D9791 and D9792 to use the updated interface once this lands

Jul 14 2014, 5:44 PM

Jul 11 2014

Firehed updated the diff for D9791: Get diviner to correctly handle contexts.
  • remove snippet from testing
Jul 11 2014, 10:52 PM
Firehed updated the diff for D9791: Get diviner to correctly handle contexts.
  • making diviner namespace aware
  • clean up a lot of the normalization and language-specific context separation logic
  • more cleanup
Jul 11 2014, 10:48 PM
Firehed updated the diff for D9792: Add namespace support in Diviner's PHP atomizer.

Remove hanging vim swapfile

Jul 11 2014, 12:45 AM
Firehed updated the diff for D9792: Add namespace support in Diviner's PHP atomizer.
  • further clean up namespace/context work
  • add php atomizer test cases and fix an issue they exposed
Jul 11 2014, 12:43 AM

Jul 10 2014

Firehed updated the diff for D9792: Add namespace support in Diviner's PHP atomizer.

Significant cleanup to NS range detection for weird files that do evil things. Fixes line numbering issue, and now includes class name in the context for method declarations

Jul 10 2014, 7:57 AM

Jul 9 2014

Firehed created T5581: Diffusion does not render Phabricator-hosted readonly mirror URIs.
Jul 9 2014, 10:15 PM · Diffusion
Firehed added a comment to D9792: Add namespace support in Diviner's PHP atomizer.

Just discovered that this screws up the line numbers from the source file since it rebuilds the tree during parsing. This feels like an incredibly wrong way to select, effectively, a few branches out of the AASTTree, but I couldn't find anything better in the source to handle it.

Jul 9 2014, 12:41 AM
Firehed updated the diff for D9792: Add namespace support in Diviner's PHP atomizer.

Perform some crazy slizing and dicing to handle a format that PHP never should have supported:

Jul 9 2014, 12:33 AM

Jul 2 2014

Firehed added a comment to D9791: Get diviner to correctly handle contexts.

Cool, thanks for the feedback!

Jul 2 2014, 1:50 AM
Firehed added inline comments to D9792: Add namespace support in Diviner's PHP atomizer.
Jul 2 2014, 1:41 AM
Firehed added inline comments to D9792: Add namespace support in Diviner's PHP atomizer.
Jul 2 2014, 1:32 AM
Firehed edited this Maniphest Task.
Jul 2 2014, 1:26 AM · Diviner, Documentation
Firehed retitled D9792: Add namespace support in Diviner's PHP atomizer from to Add namespace support in Diviner's PHP atomizer.
Jul 2 2014, 1:25 AM

Jul 1 2014

Firehed added a comment to D9791: Get diviner to correctly handle contexts.

Some screenshots of this in action:

Jul 1 2014, 9:59 PM
Firehed retitled D9791: Get diviner to correctly handle contexts from to Get diviner to correctly handle contexts.
Jul 1 2014, 9:54 PM
Firehed edited this Maniphest Task.
Jul 1 2014, 9:54 PM · Diviner, Documentation

Jun 30 2014

Firehed retitled D9786: Don't atomize closures from to Don't atomize closures.
Jun 30 2014, 6:12 PM

Jun 19 2014

Firehed added a comment to T5412: Improve "Blocking others" differential results.

Thanks @chad :)

Jun 19 2014, 12:52 AM · Differential

Jun 18 2014

Firehed created T5412: Improve "Blocking others" differential results.
Jun 18 2014, 10:32 PM · Differential

May 14 2014

Firehed added a comment to D9123: If PhabricatorIRCBot encounters a 'nick used' error, try adding a number.

(I don't have push access - need you to land this)

May 14 2014, 10:29 PM
Firehed retitled D9123: If PhabricatorIRCBot encounters a 'nick used' error, try adding a number from to If PhabricatorIRCBot encounters a 'nick used' error, try adding a number.
May 14 2014, 6:09 PM

May 10 2014

Firehed accepted D9046: Fix system agent toggling in MySQL strict mode.

Works here too :)

May 10 2014, 11:49 PM

Apr 8 2014

Firehed added a comment to D8722: Add an "Always Search" flag to the libphutil LDAP adapter.

Also tested that incorrect credentials fail. Because, you know, security. All looks good here!

Apr 8 2014, 6:29 PM
Firehed awarded D8723: Add a checkbox to the LDAP auth configuration UI to "Always Search" a Baby Tequila token.
Apr 8 2014, 6:29 PM
Firehed accepted D8723: Add a checkbox to the LDAP auth configuration UI to "Always Search".
Apr 8 2014, 6:28 PM
Firehed accepted D8722: Add an "Always Search" flag to the libphutil LDAP adapter.

Worked on my install combined with D8723 :D

Apr 8 2014, 6:28 PM
Firehed added a comment to D8159: Allow multiple LDAP search filters, and complex search queries.

The number of tickets you know off the top of your head never ceases to impress. Yeah, that's exactly it. I'll happily test a patch on our install. Thanks!

Apr 8 2014, 5:45 PM
Firehed added a comment to D8159: Allow multiple LDAP search filters, and complex search queries.

Turns out this actually killed our LDAP integration, we just didn't spot it until someone nuked their session and tried to log in again.

Apr 8 2014, 5:38 PM

Mar 25 2014

Firehed added a comment to D8611: Add array method dereference support to xhpast..

Ah, yes, that makes some amount of sense. I saw chaining_dereference: defined in the php parser that looked related, that's probably what it made work. That one gave me an unusually bad headache trying to decipher the zend states for some reason.

Mar 25 2014, 8:58 PM
Firehed edited this Maniphest Task.
Mar 25 2014, 8:36 PM · XHPAST
Firehed edited this Maniphest Task.
Mar 25 2014, 7:06 AM · XHPAST
Firehed retitled D8611: Add array method dereference support to xhpast. from to Add array method dereference support to xhpast..
Mar 25 2014, 7:06 AM

Mar 10 2014

Firehed added a comment to T4334: Support PHP5.4+ syntax in XHPAST.

I just ran into that one in practice today, so I'll probably take a crack at another grammar update to handle it in the next 48 hours or so.

Mar 10 2014, 9:22 PM · XHPAST

Feb 7 2014

epriestley awarded D8154: XHPAST array_function_derefence support a Manufacturing Defect? token.
Feb 7 2014, 6:31 PM

Feb 5 2014

Firehed edited this Maniphest Task.
Feb 5 2014, 9:33 PM · XHPAST
Firehed added a comment to T4334: Support PHP5.4+ syntax in XHPAST.

Found another issue with some array dereferencing: if ($state && !isset(self::getSearchStates()[$state])) { still fails. I'll look into that one a bit later - once I saw what the zend grammar was doing... ick.

Feb 5 2014, 9:09 PM · XHPAST
Firehed edited this Maniphest Task.
Feb 5 2014, 8:56 PM · XHPAST

Feb 4 2014

Firehed added a comment to T4334: Support PHP5.4+ syntax in XHPAST.

Update here - did spot one issue: (new Foo)->bar (added in 5.4) fails:

Feb 4 2014, 7:12 PM · XHPAST

Jan 24 2014

raguiar awarded D8054: Skip anon functions in symbol generation script a Mountain of Wealth token.
Jan 24 2014, 1:33 PM

Jan 23 2014

Firehed added a comment to T4334: Support PHP5.4+ syntax in XHPAST.

Just as an update - so far all of the changes I've tested have been working really well for 5.4 source. The stock symbol generation script seems to not mind files with more modern grammar ($x = [];, trait, use, closures, etc), and the XHPAST linter seems ok too. As you'd expect we have a massive number of files in our codebase seeing that it started close to five years ago so naturally I haven't been able to run it on everything but I'm feeling pretty good about this :)

Jan 23 2014, 10:26 PM · XHPAST

Jan 21 2014

Firehed added a comment to T4334: Support PHP5.4+ syntax in XHPAST.

This is incredible, thanks! Our devops team is going to comb through all of this and the reviews and see if we need to take a crack at anything else.

Jan 21 2014, 11:04 PM · XHPAST
Firehed claimed T4334: Support PHP5.4+ syntax in XHPAST.
Jan 21 2014, 7:59 AM · XHPAST

Jan 15 2014

Firehed added a comment to T4281: Restore background linting and unit tests for performance.

Sounds good. We're trying to push everyone through the arc workflows for everything so if it's still a legitimate issue I should have an update pretty soon.

Jan 15 2014, 9:59 PM · Arcanist
Firehed raised the priority of T4319: Remarkup text rendering bug from to Needs Triage.
Jan 15 2014, 7:57 AM · Remarkup

Jan 3 2014

Firehed added a comment to T4281: Restore background linting and unit tests for performance.

Ok, --background 0 DID allow it to go through. I don't have the stack from SIGHUP as apparently she doesn't have the pcntl extension installed either (I can get it installed if necessary, although given that your first instinct was right I'm guessing it probably isn't...)

Jan 3 2014, 12:23 AM · Arcanist

Jan 2 2014

Firehed added a comment to T4281: Restore background linting and unit tests for performance.

Ah, gotcha. I just confirmed that the developer that reported the issue to me doesn't even have xdebug installed, so it's definitely not the listener. However I told her to grab me the next time it happens (it *was* originally being run completely normally), so I'll upload a new trace with the SIGHUP trick when I have one.

Jan 2 2014, 11:54 PM · Arcanist
Firehed added a comment to T4281: Restore background linting and unit tests for performance.

Super weird. On a new branch to reproduce the issue it submitted the diff just fine (although still hangs if I arc diff --recon --no-diff manually)

Jan 2 2014, 11:38 PM · Arcanist
Firehed added a comment to T4281: Restore background linting and unit tests for performance.
eric@eric-dev ~/wepay: cat /tmp/phabricator_backtrace_511
#0 /home/sites/dev/libphutil/src/error/PhutilErrorHandler.php(188): __phutil_signal_handler__(1)
#1 [internal function]: PhutilErrorHandler::handleError(2, 'stream_select()...', '/home/sites/dev...', 196, Array)
#2 /home/sites/dev/libphutil/src/channel/PhutilChannel.php(196): stream_select(Array, Array, Array, 1, 0)
#3 /home/sites/dev/libphutil/src/channel/PhutilChannel.php(99): PhutilChannel::waitForActivity(Array, Array, Array)
#4 /home/sites/dev/libphutil/src/channel/PhutilProtocolChannel.php(135): PhutilChannel::waitForAny(Array)
#5 /home/sites/dev/libphutil/src/console/PhutilConsole.php(215): PhutilProtocolChannel->waitForMessage()
#6 /home/sites/dev/libphutil/src/console/PhutilConsole.php(117): PhutilConsole->waitForMessage()
#7 /home/sites/dev/arcanist/src/workflow/ArcanistLintWorkflow.php(530): PhutilConsole->confirm('Apply this patc...', false)
#8 /home/sites/dev/arcanist/src/workflow/ArcanistDiffWorkflow.php(1236): ArcanistLintWorkflow->run()
#9 /home/sites/dev/arcanist/src/workflow/ArcanistDiffWorkflow.php(1197): ArcanistDiffWorkflow->runLint()
#10 /home/sites/dev/arcanist/src/workflow/ArcanistDiffWorkflow.php(418): ArcanistDiffWorkflow->runLintUnit()
#11 /home/sites/dev/arcanist/scripts/arcanist.php(322): ArcanistDiffWorkflow->run()
#12 {main}
Jan 2 2014, 11:33 PM · Arcanist
Firehed added a comment to T4281: Restore background linting and unit tests for performance.

I never explicitly set it up on my machine which exhibits the problem, but I thought I saw something for eclipse running on the machine we first spotted the problem from.

Jan 2 2014, 11:29 PM · Arcanist
Firehed added a comment to T4281: Restore background linting and unit tests for performance.

It does not (I assume you mean on the arc diff command, as arc lint --background 0 just errors out with an unrecognized option)

Jan 2 2014, 10:59 PM · Arcanist
Firehed raised the priority of T4281: Restore background linting and unit tests for performance from to Needs Triage.
Jan 2 2014, 10:36 PM · Arcanist

Oct 25 2013

Firehed awarded D7368: Harbormaster v(-2) a Pterodactyl token.
Oct 25 2013, 5:50 AM

Oct 16 2013

Firehed added a comment to T3968: Differential highlighting can get thrown off by multiline comments.

Interesting. Someone on my team mentioned arc diff complaining about no content earlier, so he must have sent this one up by hand. I'd always wondered why sometimes I'd see "context not available" on diffs; I had figured it wasn't able to line something up cleanly with the previous revision/version in master/etc. Should either manually doing a git branch (as compared to arc feature) and/or pulling in changes from master daily have any impact on its use?

Oct 16 2013, 3:56 AM · Differential
Firehed raised the priority of T3968: Differential highlighting can get thrown off by multiline comments from to Needs Triage.
Oct 16 2013, 2:44 AM · Differential

Aug 21 2013

meze awarded Q33: Trigger audits against a branch? (Answer 45) a Like token.
Aug 21 2013, 8:46 AM

Jul 30 2013

Firehed added an answer to Q33: Trigger audits against a branch?.
Jul 30 2013, 10:46 PM · Audit
Firehed updated Q33: Trigger audits against a branch? from to Trigger audits against a branch?.
Jul 30 2013, 9:25 PM · Audit