rPf21f1d8ab9e909b3e2c4dc07af7a358a8336902c removed PHUIDiffTableOfContentsItemView::renderChangesetLink(). Thus call newLink() instead which should do what's wanted.
Details
- Reviewers
- None
- Group Reviewers
O1: Blessed Committers
Carefully read the code, check for existing methods in the class, study the linked changeset.
Diff Detail
- Repository
- rP Phorge
- Branch
- NEdiffToCchangesetLink (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1378 Build 1378: arc lint + arc unit
Event Timeline
The called method renderChangesetLink() is not defined anywhere so static code analysis complains.
UI-wise I could not spot anything obviously broken in the "Paths" sidebar under ToC for a merged commit, though.
If we can't find the problem, that probably means we have some dead code here that should be removed.
grep for PHUIDiffTableOfContentsItemView in the codebase to see where we might be using this class.
You have a point; I feel lazy now in hindsight, and that is deserved. :D
./src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php: final class PHUIDiffTableOfContentsItemView extends AphrontView { ./src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php: public function addItem(PHUIDiffTableOfContentsItemView $item) { ./src/applications/differential/controller/DifferentialController.php: $item = id(new PHUIDiffTableOfContentsItemView()) ./src/applications/diffusion/controller/DiffusionCommitController.php: $item = id(new PHUIDiffTableOfContentsItemView())
It seems that the function PHUIDiffTableOfContentsItemView.php::render() is not called anymore as rPf21f1d8ab9e909b3e2c4dc07af7a358a8336902c removed $rows[] = $item->render();. I cannot spot another direct render() call anywhere else in the files instantiating a PHUIDiffTableOfContentsItemView.
However, PHUIDiffTableOfContentsItemView must define render() as it's defined as an abstract method in parent AphrontView.
After emptying render(), renderChangesetMetadata() does not have any other callers either, thus also removing it.
Again, I really do not know how to test these changes so I am not convinced by this patch.