Page MenuHomePhabricator

Upgrading: Herald blocking/non-blocking reviewer rules swapped
Closed, ResolvedPublic


A patch in rPa335004a91 (20150730.herald.5.sql) incorrectly swapped "Add blocking reviewer" and "Add reviewer" rules. This patch was promoted to stable in 2015 Week 32 (August 8).

These rules are swapped back by D14061, which is part of 2015 Week 36 (Sep 5). Specifically, the patch does this:

  • Looks at the date when 20150730.herald.5.sql was applied.
  • Swaps all rules last modified before that date back to their original values.

This is probably exhaustive or nearly exhaustive in most cases. We can not be completely sure if the setting on rules modified between the patch application date and the current date is correct or swapped, and err on the side of caution by not swapping it.

You may want to review rules modified between the patch application date and the current time for correctness. You can generate a list of these rules by following these steps:

(1) Figure out when the bad patch was applied. This will tell you when rules were swapped:

SELECT applied FROM phabricator_meta_data.patch_status WHERE patch = 'phabricator:20150730.herald.5.sql';

(2) Find rules updated since then. This will tell you which rules have been updated since the patch was applied. They may need to be reviewed for correctness:

SELECT * FROM phabricator_herald.herald_rule WHERE dateModified > <TIMESTAMP>;

Replace <TIMESTAMP> with the timestamp from the first step.

Event Timeline

epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a subscriber: meitros.
epriestley added a subscriber: epriestley.
epriestley claimed this task.

This has been in stable for more than a month without any followup issues cropping up.