Page MenuHomePhabricator

T5015, Allow Herald rules for Maniphest to act on task status changes

Authored by lpriestley on Nov 12 2014, 8:17 PM.
"Mountain of Wealth" token, awarded by GMTA."Pterodactyl" token, awarded by chad."Grey Medal" token, awarded by btrahan.



Fixes T5015, Allow Herald rules for Maniphest to act on task status changes.

Test Plan

Create Herald rule for Maniphest tasks to flag a task with status "wontfix". Change status of Maniphest task to "wontfix". Task should be flagged.

Diff Detail

rP Phabricator
Lint Passed
Auto-Fixsrc/applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php:31TXT6Trailing Whitespace
Tests Passed
Build Status
Buildable 3031
Build 3035: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

lpriestley retitled this revision from to T5015, Allow Herald rules for Maniphest to act on task status changes.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.

This looks great. Issues I caught:

  • Fix trailing whitespace lint error (it should have autofixed in lint? Maybe we don't autofix that?)
  • Remove src/applications/maniphest/.DS_Store. This is an artifact created by Mac OS X. Ideally, you should set up a global .gitignore file and add .DS_Store to it, so it's ignored in all projects. GitHub has some instructions here:
  • I forgot to mention this, but when modifying an existing adapter or rule, you should bump the version number in HeraldRule (the $configVersion property). This makes sure old versions of the code (for example, running in the daemons) don't try to execute rules with the new stuff (since they won't know how to handle it). Basically makes sure this produces a sane error ("Can't execute a newer version of this rule!") instead of a confusing mess.
This revision now requires changes to proceed.Nov 12 2014, 8:24 PM
lpriestley edited edge metadata.

Bumping Herald Rule configVersion, removing DS_Store, removing trailing whitespace.

This revision is now accepted and ready to land.Nov 12 2014, 8:53 PM
This revision was automatically updated to reflect the committed changes.