Page MenuHomePhabricator

Emit more usable results from phrequent.tracking
ClosedPublic

Authored by epriestley on Jul 16 2014, 12:02 AM.

Details

Summary

I think this pretty much does what you would expect?

The "active" item is always at the top of the stack.

Test Plan

Called phrequent.tracking and got reasonable results.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

epriestley retitled this revision from to Emit more usable results from phrequent.tracking.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: hach-que.
hach-que edited edge metadata.Jul 16 2014, 8:24 AM

I hooked up my patch to use this, and observed these incorrect calculations:

[arcpatch-D7327] james@james-laptop:~/Projects/Phabricator/arcanist> arc --conduit-uri=http://phabricator.local/ time
Status        Tracked Name          
(In Progress)     6 s T2: test again
(Suspended)       1 m D8: blah      
(Suspended)       4 s D7: blah      
[arcpatch-D7327] james@james-laptop:~/Projects/Phabricator/arcanist> arc --conduit-uri=http://phabricator.local/ stop D8
Stopped:  D8: blah

Status        Tracked Name          
(In Progress)    10 s T2: test again
(Suspended)       1 m D7: blah

After stopping D8, D7 shows an increase to 1 minute, but it should show 4 seconds instead.

hach-que added inline comments.Jul 16 2014, 8:26 AM
src/applications/phrequent/conduit/ConduitAPI_phrequent_tracking_Method.php
34

I think this is the cause of the 1 minute issue, because the calculations need to know about the stopped D8 entry.

hach-que added inline comments.Jul 16 2014, 8:27 AM
src/applications/phrequent/conduit/ConduitAPI_phrequent_tracking_Method.php
34

Actually removing this doesn't fix it (it just makes things worse).

epriestley edited edge metadata.
  • Fix bug which would cause ended, preempting events to not be considered preempting.
  • Add some test coverage.
  • Rename file.
hach-que accepted this revision.Jul 16 2014, 9:38 PM
hach-que edited edge metadata.

Tested this and got the intended results. D7327 has already been updated to use this new API.

This revision is now accepted and ready to land.Jul 16 2014, 9:38 PM
epriestley closed this revision.Jul 17 2014, 12:12 AM
epriestley updated this revision to Diff 23890.

Closed by commit rPab3c17a2cd97 (authored by @epriestley).