Move rendering to SearchEngine for Calendar
ClosedPublic
Actions

Authored by epriestley on May 8 2014, 4:22 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T4986: Implement new Dashboard panel types

Commits

Restricted Diffusion Commit
rP684805a88a16: Move rendering to SearchEngine for Calendar

Summary

Ref T4986. This one needs getApplicationURI() so make it a little beefier to deal with that.

(It would be vaguely nice to somehow share the handle and application stuff between Controllers and Engine classes like this, but I don't immediately see a clean way to do it without traits. Not a big deal, in any case.)

Test Plan

Viewed Calendar.
Made a Calendar panel.
Viewed feed.

Diff Detail

Repository

rP Phabricator

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

epriestley updated this revision to Diff 21401.May 8 2014, 4:22 PM

epriestley retitled this revision from to Move rendering to SearchEngine for Calendar.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a task: T4986: Implement new Dashboard panel types.

Herald added a subscriber: epriestley. · View Herald TranscriptMay 8 2014, 4:22 PM

Harbormaster completed remote builds in B303: Diff 21401.May 8 2014, 4:23 PM

re: cleanliness without traits - as is seems pretty good to me since someone adding code here won't really care or need to know about the base implementation, and the base implementations seem pretty static to me (so low maintenance to maintain.)

Still, I think we should be careful to DRY as we add functionality. One thing I haven't seen yet but could be an example of an opportunity is rendering an imploded list of handles - I believe some of that code is duplicated places, and it instead could be centralized on the handle data object or some handle rendering object.

This revision is now accepted and ready to land.May 8 2014, 7:05 PM