Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7604: Remove "Arcanist Projects"
- Commits
- Restricted Diffusion Commit
rP635ea2cbaf91: Remove arcanist projects from Herald
See D12957.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This doesn't actually work as is... the following exception is thrown when submitting a diff:
ERR-CONDUIT-CORE: Unknown field 'arcanist-project'!
I think that the real question here is what to do with any existing Herald rules that use Arcanist projects? There's a few options:
- Change these Herald rules to be repository-based with a migration.
- Destroy the invalid Herald rules with a migration.
- Make Herald more resilient against invalid keys.
Make Herald more resilient against invalid keys.
I favor doing this. Now that Herald CustomActions are possible and fields are creeping slowly toward being modular, there are more ways that users could write invalid rules than there were in the past, and I anticipate it continuing to become easier in the future.
Specifically, I'd propose this behavior:
- If a rule has an invalid field, the conditions fail and the actions do not execute.
- The transcript shows that the rule failed because of an invalid field, and points at the issue.
- If a rule has an invalid action, that action fails but other actions execute (when we can detect this early, I'd slightly prefer failing all actions, but we can't do anything about that if the actual apply() code fails).
- The transcript shows that the action failed.
- Everything else (particularly, other rules) continues normally in both cases.
- Ideally, the UI should call this stuff out too when viewing or editing rules, although that isn't terribly important and could wait until the next Herald UI pass.
I guess we could write a migration to convert "Arcanist Project" Herald rules into "Repository Callsign" Herald rules, but I'm not sure if this is worth pursuing.
IMO nah, D12957 should have done enough - after all transcripts will show reasons for failure of rule.