Depends on D9906.
This adds arc start, arc stop and arc tracking for tracking tasks, diffs and other objects in Phrequent.
Details
Details
- Reviewers
skyronic epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T3569: Make Phrequent somewhat usable
T3969: Phrequent time tracking via arcanist - Commits
- rARCf658f170808e: Add Phrequent workflows to Arcanist
Tested this against a local install.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- arcpatch-D7327
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1716 Build 1717: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
This seems pretty cumbersome. You have to type:
arc track --start --task T123
Is there a reason you went with that instead of:
arc start T123
...as in D7326?
Generally, I think the examples in D7326 (arc start, arc stop) are a lot easier to use than this. You can implement them like this:
- Accept a wildcard argument, which matches the T123 part.
- Throw if there are zero args, or more than one arg ("specify exactly one object to start work on").
- Use the phid.lookup method to look up the object the user names. This will convert a string like T123 into an object PHID.
- Call phrequent.start on the PHID.
The --task stuff is both cumbersome and not general. Phrequent supports tracking any object type, including types introduced by third-party applications. If you use phid.lookup, all of this works for free. If you don't, you'll have to implement --paste, --revision, --commit, --mock, etc., for every object type, and third-party applications won't be accessible from the CLI.
Comment Actions
Looks like this is this parallel work to D7326, right? Should one of these be finished?
Comment Actions
The author of these changes is a third-party contributor, so presumably he just hasn't found the time to finish them.
We'll build support for these things in the upstream eventually even if he never picks these back up, but Phrequent is a very low priority right now.
| src/workflow/ArcanistStartWorkflow.php | ||
|---|---|---|
| 93–94 | Because the current timestamp is the same as the time that just ended, it can't distinguish between a user time that's currently running (where end time == now) and a user time that's been ended at the current time (where end time == now as well). We should probably fix this inside Phrequent at some point, but all of the strata and time block calculations stuff is complex and I don't want to break it. | |
Comment Actions
What do you think about arc time instead of arc tracking? Offhand, it's a little shorter and seems a little more closely related...
Couple of minor inlines. This mostly looks good.
| src/workflow/ArcanistStartWorkflow.php | ||
|---|---|---|
| 41–46 | Is there any reason to have this? | |
| 56–57 | This permits arc start T1 T2 but ignores T2. This also permits arc start --object T1 T2 but ignores T1. | |
| 69–70 | throw new ArcanistUsageException(...) | |
| 75–85 | These calls could use futures, although probably not worth bothering. | |
| 93–94 | Sleeping seems pretty bad. We should find another way to get around this for now so users arent' stuck watiing. | |
| src/workflow/ArcanistStopWorkflow.php | ||
| 61–66 | As above on arg handling. | |
| src/workflow/ArcanistTrackingWorkflow.php | ||
| 6–8 ↗ | (On Diff #23798) | It would be slightly nicer to make this abstract and extend all three (start, stop, tracking) from it. This workflow isn't inherently any more core. |
| 89–99 ↗ | (On Diff #23798) | Can we just fake this instead of doing the sleeps? |
| 101–107 ↗ | (On Diff #23798) | Use PhutilConsoleTable |
| src/workflow/ArcanistPhrequentWorkflow.php | ||
|---|---|---|
| 63 | Given the way the API currently works, this will never print 'Stopped' and always print 'in progress' on the first item. Maybe just a "*" or something to reinforce that the top item is the active one? This is easy to tweak later. | |