Changeset View
Changeset View
Standalone View
Standalone View
src/infrastructure/edges/constants/PhabricatorEdgeConfig.php
| Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Lines | final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { | ||||
| const TYPE_PANEL_HAS_DASHBOARD = 46; | const TYPE_PANEL_HAS_DASHBOARD = 46; | ||||
| const TYPE_OBJECT_HAS_WATCHER = 47; | const TYPE_OBJECT_HAS_WATCHER = 47; | ||||
| const TYPE_WATCHER_HAS_OBJECT = 48; | const TYPE_WATCHER_HAS_OBJECT = 48; | ||||
| const TYPE_OBJECT_NEEDS_SIGNATURE = 49; | const TYPE_OBJECT_NEEDS_SIGNATURE = 49; | ||||
| const TYPE_SIGNATURE_NEEDED_BY_OBJECT = 50; | const TYPE_SIGNATURE_NEEDED_BY_OBJECT = 50; | ||||
| /* !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! */ | |||||
joshuaspence: For consistency:
```lang=diff
-/* !!! STOP !!! STOP !!! STOP !!! STOP !!! STOP !!! STOP !!! | |||||
Not Done Inline ActionsDisregard this... there was method to your madness. joshuaspence: Disregard this... there was method to your madness. | |||||
Not Done Inline ActionsHaha, I was pretty sad about this. Maybe I can come up with some alternate art which fits more cleanly. epriestley: Haha, I was pretty sad about this. Maybe I can come up with some alternate art which fits more… | |||||
Not Done Inline ActionsHooray! joshuaspence: Hooray! | |||||
| // HEY! DO NOT ADD NEW CONSTANTS HERE! | |||||
| // Instead, subclass PhabricatorEdgeType. | |||||
| /* !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! STOP !!!! */ | |||||
Not Done Inline ActionsAs above. joshuaspence: As above. | |||||
Not Done Inline Actionsderp joshuaspence: derp | |||||
| const TYPE_TEST_NO_CYCLE = 9000; | const TYPE_TEST_NO_CYCLE = 9000; | ||||
| const TYPE_PHOB_HAS_ASANATASK = 80001; | const TYPE_PHOB_HAS_ASANATASK = 80001; | ||||
| const TYPE_ASANATASK_HAS_PHOB = 80000; | const TYPE_ASANATASK_HAS_PHOB = 80000; | ||||
| const TYPE_PHOB_HAS_ASANASUBTASK = 80003; | const TYPE_PHOB_HAS_ASANASUBTASK = 80003; | ||||
| const TYPE_ASANASUBTASK_HAS_PHOB = 80002; | const TYPE_ASANASUBTASK_HAS_PHOB = 80002; | ||||
| const TYPE_PHOB_HAS_JIRAISSUE = 80004; | const TYPE_PHOB_HAS_JIRAISSUE = 80004; | ||||
| const TYPE_JIRAISSUE_HAS_PHOB = 80005; | const TYPE_JIRAISSUE_HAS_PHOB = 80005; | ||||
Not Done Inline Actionsarray_diff() takes array parameters, so $exclude may as well have an array type hint. joshuaspence: `array_diff()` takes `array` parameters, so `$exclude` may as well have an `array` type hint. | |||||
Not Done Inline ActionsIt's not clear to me (from the context of this diff alone... it will possibly become clearer in a later diff) what this function is used for. Unless it becomes clear in a later diff, perhaps a docblock comment would help. joshuaspence: It's not clear to me (from the context of this diff alone... it will possibly become clearer in… | |||||
Not Done Inline ActionsYeah, this could be more clear. In future diffs, I start replacing stuff like this: $inverse = PhabricatorEdgeConfig::getInverse($const); ...with this: $type_obj = PhabricatorEdgeType::getByConst($const); $inverse = $type_obj->getInverseEdgeConstant(); For that to work, we either need to convert every edge type up front (which is a lot of tedious work, and creates some level of unnecessary risk) or expose all the existing data through as PhabricatorEdgeTypes. This method enables the latter, by building legacy objects for all the existing edge types, so the migration can take place gradually but new code can pretend the migration is already complete because the old types look like they're new types. epriestley: Yeah, this could be more clear. In future diffs, I start replacing stuff like this:
$inverse… | |||||
| /** | |||||
| * Build @{class:PhabricatorLegacyEdgeType} objects for edges which have not | |||||
Not Done Inline ActionsPluralize "edge". joshuaspence: Pluralize "edge". | |||||
Not Done Inline Actionss/edge/edges/ joshuaspence: `s/edge/edges/` | |||||
| * yet been modernized. This allows code to act as though we've completed | |||||
| * the edge type migration before we actually do all the work, by building | |||||
| * these fake type objects. | |||||
| * | |||||
| * @param list<const> List of edge types that objects should not be built for. | |||||
Not Done Inline Actions"That objects" is repeated. joshuaspence: "That objects" is repeated. | |||||
| * This is used to avoid constructing duplicate objects for edge constants | |||||
| * which have migrated and already have a real object. | |||||
| * @return list<PhabricatorLegacyEdgeType> Real-looking edge type objects for | |||||
| * unmigrated edge types. | |||||
| */ | |||||
| public static function getLegacyTypes(array $exclude) { | |||||
| $consts = array_merge( | |||||
| range(1, 50), | |||||
| array(9000), | |||||
| range(80000, 80005)); | |||||
| $consts = array_diff($consts, $exclude); | |||||
| $map = array(); | |||||
| foreach ($consts as $const) { | |||||
| $prevent_cycles = self::shouldPreventCycles($const); | |||||
| $inverse_constant = self::getInverse($const); | |||||
| $map[$const] = id(new PhabricatorLegacyEdgeType()) | |||||
| ->setEdgeConstant($const) | |||||
| ->setShouldPreventCycles($prevent_cycles) | |||||
| ->setInverseEdgeConstant($inverse_constant) | |||||
| ->setStrings( | |||||
| array( | |||||
| self::getAddStringForEdgeType($const), | |||||
| self::getRemoveStringForEdgeType($const), | |||||
| self::getEditStringForEdgeType($const), | |||||
| self::getFeedStringForEdgeType($const), | |||||
| )); | |||||
| } | |||||
| return $map; | |||||
| } | |||||
| public static function getInverse($edge_type) { | public static function getInverse($edge_type) { | ||||
| static $map = array( | static $map = array( | ||||
| self::TYPE_TASK_HAS_COMMIT => self::TYPE_COMMIT_HAS_TASK, | self::TYPE_TASK_HAS_COMMIT => self::TYPE_COMMIT_HAS_TASK, | ||||
| self::TYPE_COMMIT_HAS_TASK => self::TYPE_TASK_HAS_COMMIT, | self::TYPE_COMMIT_HAS_TASK => self::TYPE_TASK_HAS_COMMIT, | ||||
| self::TYPE_TASK_DEPENDS_ON_TASK => self::TYPE_TASK_DEPENDED_ON_BY_TASK, | self::TYPE_TASK_DEPENDS_ON_TASK => self::TYPE_TASK_DEPENDED_ON_BY_TASK, | ||||
| self::TYPE_TASK_DEPENDED_ON_BY_TASK => self::TYPE_TASK_DEPENDS_ON_TASK, | self::TYPE_TASK_DEPENDED_ON_BY_TASK => self::TYPE_TASK_DEPENDS_ON_TASK, | ||||
| Show All 31 Lines | static $map = array( | ||||
| self::TYPE_FILE_HAS_OBJECT => self::TYPE_OBJECT_HAS_FILE, | self::TYPE_FILE_HAS_OBJECT => self::TYPE_OBJECT_HAS_FILE, | ||||
| self::TYPE_ACCOUNT_HAS_MEMBER => self::TYPE_MEMBER_HAS_ACCOUNT, | self::TYPE_ACCOUNT_HAS_MEMBER => self::TYPE_MEMBER_HAS_ACCOUNT, | ||||
| self::TYPE_MEMBER_HAS_ACCOUNT => self::TYPE_ACCOUNT_HAS_MEMBER, | self::TYPE_MEMBER_HAS_ACCOUNT => self::TYPE_ACCOUNT_HAS_MEMBER, | ||||
| self::TYPE_DREV_HAS_COMMIT => self::TYPE_COMMIT_HAS_DREV, | self::TYPE_DREV_HAS_COMMIT => self::TYPE_COMMIT_HAS_DREV, | ||||
| self::TYPE_COMMIT_HAS_DREV => self::TYPE_DREV_HAS_COMMIT, | self::TYPE_COMMIT_HAS_DREV => self::TYPE_DREV_HAS_COMMIT, | ||||
| self::TYPE_OBJECT_HAS_CONTRIBUTOR => self::TYPE_SUBSCRIBED_TO_OBJECT, | self::TYPE_OBJECT_HAS_CONTRIBUTOR => self::TYPE_CONTRIBUTED_TO_OBJECT, | ||||
| self::TYPE_CONTRIBUTED_TO_OBJECT => self::TYPE_OBJECT_HAS_CONTRIBUTOR, | self::TYPE_CONTRIBUTED_TO_OBJECT => self::TYPE_OBJECT_HAS_CONTRIBUTOR, | ||||
| self::TYPE_TASK_HAS_MOCK => self::TYPE_MOCK_HAS_TASK, | self::TYPE_TASK_HAS_MOCK => self::TYPE_MOCK_HAS_TASK, | ||||
| self::TYPE_MOCK_HAS_TASK => self::TYPE_TASK_HAS_MOCK, | self::TYPE_MOCK_HAS_TASK => self::TYPE_TASK_HAS_MOCK, | ||||
| self::TYPE_PHOB_HAS_ASANATASK => self::TYPE_ASANATASK_HAS_PHOB, | self::TYPE_PHOB_HAS_ASANATASK => self::TYPE_ASANATASK_HAS_PHOB, | ||||
| self::TYPE_ASANATASK_HAS_PHOB => self::TYPE_PHOB_HAS_ASANATASK, | self::TYPE_ASANATASK_HAS_PHOB => self::TYPE_PHOB_HAS_ASANATASK, | ||||
| ▲ Show 20 Lines • Show All 381 Lines • Show Last 20 Lines | |||||
For consistency: