Page MenuHomePhabricator

Guarantee that fields copied from diffs persist on revisions
ClosedPublic

Authored by epriestley on Mar 24 2014, 5:50 PM.
Tags
None
Referenced Files
F15528451: D8610.id20400.diff
Tue, Apr 22, 11:15 AM
F15523323: D8610.id.diff
Mon, Apr 21, 12:38 AM
F15520716: D8610.id20419.diff
Sun, Apr 20, 7:31 AM
F15518596: D8610.diff
Sat, Apr 19, 2:53 PM
F15516228: D8610.diff
Fri, Apr 18, 5:28 PM
F15511962: D8610.id20419.diff
Thu, Apr 17, 7:01 AM
F15469099: D8610.diff
Fri, Apr 4, 9:00 AM
F15456219: D8610.diff
Sun, Mar 30, 7:55 AM

Details

Summary

Fixes T4636. Currently, we copy fields from the diff to the revision during the external effect phase, but there's no guarantee that we persist the object after this phase.

(In practice, when Herald rules trigger they cause the object to persist on this install, which is why we don't see this issue.)

Instead, move the field copies to the internal phase, where persistence is guaranteed.

Also consolidate some of the diff loading.

Test Plan

Ran arc diff.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Guarantee that fields copied from diffs persist on revisions.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Mar 25 2014, 8:03 PM
epriestley updated this revision to Diff 20419.

Closed by commit rP2547a222d8b1 (authored by @epriestley).