Ref T5245. Updates the project/object edge to use a modern class definition. Moves further toward real edges.
Details
Details
- Reviewers
joshuaspence btrahan chad - Maniphest Tasks
- T5245: Migrate Maniphest Projects to use edge infrastructure
- Commits
- Restricted Diffusion Commit
rP33120e377a63: Modernize Project/Object edges
Added projects to some objects, viewed transactions in transaction record.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- projedge8
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1572 Build 1573: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
This looks good. Again there are a couple of minor issues which are carried over from previous diffs.
| src/applications/differential/customfield/DifferentialProjectsField.php | ||
|---|---|---|
| 38 | As in D9848, this feels awkward. | |
| 101 | As above. | |
| src/applications/diffusion/controller/DiffusionRepositoryController.php | ||
| 167 | As above. | |
| src/applications/diffusion/controller/DiffusionRepositoryEditBasicController.php | ||
| 76 | As above. | |
| src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php | ||
| 268 | You get the idea... | |
| src/applications/project/edge/PhabricatorProjectEdgeTypeObjectHasProject.php | ||
| 3 ↗ | (On Diff #23620) | As in D9839, I'm not a fan of the class name. |
| 6–11 ↗ | (On Diff #23620) | As in D9839, this feels sorta awkward. |
| 42 ↗ | (On Diff #23620) | As in D9839, I'm not convinced that we need a $total_count parameter. |
| 78–83 ↗ | (On Diff #23620) | As in D9839, the parameter order is wrong. |
| 89 ↗ | (On Diff #23620) | As above. |
| src/applications/project/edge/PhabricatorProjectEdgeTypeProjectHasObject.php | ||
| 3 ↗ | (On Diff #23620) | As above. |
| 6–10 ↗ | (On Diff #23620) | As above. |
| src/applications/differential/customfield/DifferentialProjectsField.php | ||
|---|---|---|
| 101 | This is maybe an argument against using objects. This is getting serialized and must be a scalar. Or we need to build a lot more "stuff" around edge transactions, which might not be terrible, but they only feel a touch clumsy to me right now (and mostly during construction). | |