Page MenuHomePhabricator

Move many "reviewers" readers to new storage
ClosedPublic

Authored by epriestley on Mar 20 2017, 8:24 PM.
Tags
None
Referenced Files
F15568357: D17517.id42126.diff
Sat, May 3, 12:51 AM
F15549694: D17517.id42126.diff
Sun, Apr 27, 7:22 AM
F15549643: D17517.id42141.diff
Sun, Apr 27, 7:03 AM
F15548303: D17517.id.diff
Sat, Apr 26, 10:52 PM
F15540033: D17517.diff
Fri, Apr 25, 6:26 AM
F15514681: D17517.id42126.diff
Fri, Apr 18, 4:30 AM
F15483370: D17517.id42141.diff
Wed, Apr 9, 11:58 AM
F15454853: D17517.diff
Mar 29 2025, 8:51 PM
Subscribers
None

Details

Summary

Ref T10967.

When we query for revisions with particular reviewers, use the new table to drive the query.

When we load revisions for use in the application, also use the new table to drive the query.

This doesn't convert everything: there's some old loadRelationships() stuff still using the old table. But this moves the major stuff over.

(This also changes the icon for "commented" from a question mark to a speech bubble.)

Test Plan
  • Viewed revision lists and detail views on old and new code, saw identical outcomes.
  • Updated revisions, accepted/rejected/commented on revisions.
  • Hit the "Accepted Older" and "Commented Older" states by taking an action and then updating.
  • Grepped for removed methods (like getEdgeData() and getDiffID()).

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad added inline comments.
src/applications/differential/customfield/DifferentialCustomField.php
78

?

This revision is now accepted and ready to land.Mar 20 2017, 9:47 PM
src/applications/differential/customfield/DifferentialCustomField.php
78

It's possible that getActiveDiff() isn't loaded in some cases so we'd get a "thing not attached" exception. Since all we need it for is to pretty-up the UI I'm just blarpin' out.

This revision was automatically updated to reflect the committed changes.