Page MenuHomePhabricator

Remove calls to methods deprecated by newPage()
Open, NormalPublic

Description

D14382 fully deprecates many older ways of doing things. These aren't blocking anything, but should be modernized sooner or later. Specifically, these things are deprecated:


PhabricatorController->buildStandardPageResponse() (~15 callsites)

This is a very old method which was already effectively obsoleted by buildApplicationPage(). Callsites should be replaced with:

return $this->newPage()
  ->...

PhabricatorController->buildStandardPageView() (~12 callsites)

This is a very, very old method which was obsoleted by (or before?) buildApplicationPage(). Callsites should be replaced with:

return $this->newPage()
  ->...

PhabricatorController->buildApplicationPage() (~250 callsites)

This is the modern method which is now deprecated, although I don't expect us to get rid of it for a while. It should be replaced with:

return $this->newPage()
  ->...

*->buildSideNavView() (~175 callsites)

Almost all of these should probably be removed. The new code should have the same effect in most cases without requiring this method.

Some of these do extra stuff, and we'll need to make changes to accommodate that.


*->buildApplicationMenu() (~40 callsites)

Many of these can likely be simplified.


new PhabricatorApplicationSearchController() (~80 callsites)

All of these can almost certainly be simplified now.

Related Objects

StatusAssignedTask
OpenNone
Resolvedepriestley
OpenNone
OpenNone
Openepriestley
Resolvedepriestley
Wontfixepriestley
Resolvedbtrahan
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
DuplicateNone
ResolvedNone
Resolvedepriestley
ResolvedNone
Resolvedepriestley
OpenNone
InvalidNone
ResolvedNone
Resolvedepriestley
OpenNone

Event Timeline

epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a project: Infrastructure.
epriestley added subscribers: chad, epriestley.

Let me get a little further with ApplicationEditor and throw a couple of reference diffs up here before anyone goes crazy with this, since I might still need to change some APIs.

When we make a hire we can make them do this "to become more familiar with Phabricator's codebase".

bwahaha

This will "help" you "become more familiar with Phabricator's codebase".

This task discusses replacing old callsites with new calls, although much of the work has been done in the intervening time. Here's my best effort at figuring out what's still going on:

  • buildStandardPageResponse() looks like it's only present in XHProf and XHPAST. XHProf should be easy to fix, although it may be tricky to test (since you need to install the xhprof PHP extension and configure a bunch of debug stuff). XHPAST may be tricky to fix because it uses weird iframe stuff that doesn't exist anywhere else. If possible, these should be replaced with newPage().
  • buildStandardPageView(), same deal.
  • buildApplicationPage() appears to have zero remaining callsites. The method can be removed from PhabricatorController.

The SideNavView, ApplicationMenu and PhabricatorApplicationSearchController stuff probably isn't worth fixing in a dedicated way. They still have a ton of callsites, they're more complicated, and they have been getting cleaned up gradually.

Basically, do whatever's easy here and then we can just close it or I can take a stab at fixing the weird iframe stuff. This is just a few simple mechanical fixes that you can use to rack up valuable contributor points as you become more familiar with things.

When converted, all of the new behavior should be exactly identical to the old behavior.

The document Using XHProf may be helpful in testing the XHProf application. Installing XHProf and getting it running will help with doing performance-related work in the future.

epriestley added a subscriber: jcox.