Page MenuHomePhorge

Fix call to non-existing renderChangesetLink() in PHUIDiffTableOfContentsItemView
Needs ReviewPublic

Authored by aklapper on May 17 2024, 13:13.

Details

Reviewers
None
Group Reviewers
O1: Blessed Committers
Summary

rPf21f1d8ab9e909b3e2c4dc07af7a358a8336902c removed PHUIDiffTableOfContentsItemView::renderChangesetLink(). Thus call newLink() instead which should do what's wanted.

Test Plan

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

Is there something that's not working here?

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.

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.