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
F18859123: D8610.id.diff
Sun, Nov 2, 12:12 AM
F18853362: D8610.diff
Fri, Oct 31, 4:00 PM
F18754188: D8610.diff
Oct 4 2025, 9:45 PM
F18734711: D8610.id.diff
Sep 30 2025, 11:00 PM
F18702496: D8610.id.diff
Sep 28 2025, 12:20 AM
F18688671: D8610.diff
Sep 26 2025, 4:59 PM
F18670447: D8610.id20419.diff
Sep 25 2025, 12:21 AM
F18107142: D8610.diff
Aug 10 2025, 11:42 PM

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).