HomePhabricator

Put all DifferentialComment loading behind DifferentialCommentQuery

Description

Put all DifferentialComment loading behind DifferentialCommentQuery

Summary:
Ref T2222.

I'm thinking about how I want to approach the Asana sync, and I want to try to do T2222 first so that we can build it cleanly on top of ApplicationTransactions. I think we can at least walk down this road a little bit and if it turns out to be scary we can take another approach.

I was generally very happy with how the auth migration turned out (seemingly, it was almost completely clean), and want to pursue a similar strategy here. Basically:

  • Wrap the new objects in the old objects for reads/writes.
  • Migrate all the existing data to the new table.
  • Everything hard is done; move things over a piece at a time at a leisurely pace in lots of smallish, relatively-easy-to-understand changes.

This deletes or abstracts all reads of the DifferentialComment table. In particular, these things are deleted:

  • The script undo_commits.php, which I haven't pointed anyone at in a very long time.
  • The differential.getrevisionfeedback Conduit method, which has been marked deprecated for a year or more.
  • The /stats/ interface in Differential, which should be rebuilt on Fact and has never been exposed in the UI. It does a ton of joins and such which are prohibitively difficult to migrate.

This leaves a small number of reading interfaces, which I replaced with a new DifferentialCommentQuery. Some future change will make this actually load transactions and wrap them with DifferentialComment interfaces.

Test Plan: Viewed a revision; made revision comments

Reviewers: btrahan

Reviewed By: btrahan

CC: edward, chad, aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D6260

Details

Provenance
epriestleyAuthored on Jun 21 2013, 7:51 PM
Reviewer
btrahan
Differential Revision
Restricted Differential Revision
Parents
rP0a044ef27571: Make old GitHub OAuth URIs work for now
Branches
Unknown
Tags
Unknown
Tasks
T2222: Implement ApplicationTransactions in Differential

Event Timeline