Page MenuHomePhabricator
Differential D17063

Introduce ModularTransactionType::isRenderingTargetExternal
ClosedPublic
Actions

Authored by avivey on Dec 16 2016, 12:15 AM.
Tags
None
Referenced Files
F15722519: D17063.diff
Thu, May 29, 8:53 AM
Unknown Object (File)
Tue, May 20, 11:27 AM
Unknown Object (File)
Sat, May 10, 6:15 PM
Unknown Object (File)
Wed, May 7, 3:03 AM
Unknown Object (File)
Apr 23 2025, 8:40 PM
Unknown Object (File)
Apr 20 2025, 12:13 PM
Unknown Object (File)
Apr 19 2025, 10:28 AM
Unknown Object (File)
Apr 18 2025, 7:55 PM
View All Files
Subscribers
Korvin

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
rP8b7e99f68c97: Introduce ModularTransactionType::isRenderingTargetExternal
Summary

This is just some housekeeping - see note in D16287. Basically, "isTextMode" doesn't convey enough information.

Test Plan

git grep isTextMode | grep -v Remarkup, and visit all callsites; There are 4 of them left.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

avivey updated this revision to Diff 41050.Dec 16 2016, 12:15 AM
avivey retitled this revision from to Introduce ModularTransactionType::isRenderingTargetExternal.
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey added a reviewer: epriestley.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptDec 16 2016, 12:15 AM
Herald added a subscriber: Korvin. · View Herald Transcript
avivey added a comment.Dec 16 2016, 12:20 AM

Of the 4 remaining callsites, 3 are in this file (1 for actual rendering, and 2 for $viewer->renderHandle()->setAsText(), which we might want to someday extend to setTargetExternal too), and one is in DifferentialRevisionReviewersTransaction which is also doing rendering, but looks funny non-the-less (implode vs phutil_implode_html).

I've also grepped for getRenderingTarget, and it like all callsites really are concerned with rendering.

epriestley accepted this revision.Dec 16 2016, 12:47 AM
epriestley edited edge metadata.

The DifferentialRevisionReviewersTransaction one is maybe hopefully temporary-ish.

This revision is now accepted and ready to land.Dec 16 2016, 12:47 AM
avivey edited the test plan for this revision. (Show Details)Dec 16 2016, 12:51 AM
Closed by commit rP8b7e99f68c97: Introduce ModularTransactionType::isRenderingTargetExternal (authored by avivey, committed by avivey). · Explain WhyDec 16 2016, 12:52 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
applications/
transactions/
storage/
14 lines

Diff 41051

View Options

src/applications/transactions/storage/PhabricatorModularTransactionType.php

Loading...
Phabricator · Built by Phacility