Page MenuHomePhabricator

Remove obsolete "relationships" code from Differential
ClosedPublic

Authored by epriestley on Mar 20 2017, 9:10 PM.
Tags
None
Referenced Files
F15463120: D17518.id42142.diff
Tue, Apr 1, 8:29 PM
F15462460: D17518.diff
Tue, Apr 1, 3:13 PM
F15455272: D17518.id42128.diff
Sat, Mar 29, 11:14 PM
F15437522: D17518.id.diff
Tue, Mar 25, 8:04 PM
F15434584: D17518.id.diff
Tue, Mar 25, 4:33 AM
F15431268: D17518.diff
Mon, Mar 24, 11:27 AM
F15424927: D17518.id42128.diff
Sun, Mar 23, 1:10 AM
F15377671: D17518.id42128.diff
Thu, Mar 13, 10:08 AM
Subscribers
None

Details

Summary

Ref T10967. There have been two different ways to load reviewers for a while: needReviewerStatus() and needRelationships().

The needRelationships() stuff was a false start along time ago that didn't really go anywhere. I believe the idea was that we might want to load several different types of edges (subscribers, reviewers, etc) on lots of different types of objects. However, all that stuff pretty much ended up modularizing so that main Query classes did not need to know about it, so needRelationships() never got generalized or went anywhere.

A handful of things still use it, but get rid of them: they should either needReviewerStatus() to get reviewer info, or the ~3 callsites that care about subscribers can just load them directly.

Test Plan
  • Grepped for removed methods (needRelationships(), getReviewers(), getCCPHIDs(), etc).
  • Browsed Diffusion, Differential.
  • Called differential.query.

It's possible I missed some stuff, but it should mostly show up as super obvious fatals ("call needReviewerStatus() before getReviewerStatus()!").

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Mar 20 2017, 10:09 PM
This revision was automatically updated to reflect the committed changes.