Add chart functions "x()" and "constant(3)"
ClosedPublic
Actions

Authored by epriestley on Apr 17 2019, 2:04 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13279: Build Charting for Facts

Commits

rPc4e4448ee649: Add chart functions "x()" and "constant(3)"

Summary

Depends on D20442. Ref T13279. Add basic support for drawing chart functions that are not based on Facts first-party ETL datasets. Some general goals:

This might be useful to draw a line like "goal" or "profitability".
This might be useful to pull data from an external source.
For composable functions like "add" or "subtract", which are useful in manipulating ETL datasets, these value functions will make testing easier.

Test Plan

Added a constant(256) function:

Screen Shot 2019-04-17 at 7.00.27 AM.png (1×1 px, 189 KB)

Diff Detail

Repository

rP Phabricator

Branch

chart7

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22620
Build 31003: Run Core Tests
Build 31002: arc lint + arc unit

Event Timeline

epriestley created this revision.Apr 17 2019, 2:04 PM

Herald added a subscriber: yelirekim. · View Herald TranscriptApr 17 2019, 2:04 PM

Harbormaster completed remote builds in B22620: Diff 48750.Apr 17 2019, 2:06 PM

epriestley requested review of this revision.Apr 17 2019, 2:06 PM

epriestley added a child revision: D20444: Add a "sin()" function to charts.Apr 17 2019, 2:28 PM

amckinley accepted this revision.Apr 17 2019, 11:16 PM

amckinley added inline comments.

src/applications/fact/chart/PhabricatorConstantChartFunction.php
30	Not sure how I feel about passing `$limit` here. Won't every chart function basically copy/paste the rest of the code in this function?
32–33	Also it's not obvious to me why `$limit` gets passed as an argument but the axis is an instance variable on this object.
src/applications/fact/controller/PhabricatorFactChartController.php
24	This is setting the default argument to 256, right? I was looking for some reason why this to be a power of 2, maybe pick a different constant like 42?

This revision is now accepted and ready to land.Apr 17 2019, 11:16 PM

epriestley added inline comments.Apr 18 2019, 8:15 PM

src/applications/fact/chart/PhabricatorConstantChartFunction.php
32–33	Yeah, this changes in the ~next diff.
src/applications/fact/controller/PhabricatorFactChartController.php
24	This is just drawing a visible line on the "demo" chart. 256 just happened to produce a reasonable line in the middle of the data on my local instance.