Paths

Table of Contentst

Differential D20445

Make chart function argument parsing modular/flexible with 900 pages of error messages
ClosedPublic
Actions

Authored by epriestley on Apr 17 2019, 6:29 PM.

Tags

None

Referenced Files

	F13268336: D20445.diff
	Wed, May 29, 4:55 AM

	F13253848: D20445.diff
	Sat, May 25, 2:59 AM

	F13233408: D20445.diff
	Tue, May 21, 2:16 AM

	F13220394: D20445.diff
	Sun, May 19, 1:20 AM

	F13211394: D20445.diff
	Fri, May 17, 5:43 AM

	F13196630: D20445.diff
	Sun, May 12, 11:25 PM

	Unknown Object (File)
	Tue, May 7, 1:52 AM

	Unknown Object (File)
	Mon, May 6, 7:37 PM

Subscribers

Details

Reviewers

Maniphest Tasks

T13279: Build Charting for Facts

Commits

rPedaf17f3fe51: Make chart function argument parsing modular/flexible with 900 pages of error…

Summary

Depends on D20444. Ref T13279. Instead of ad-hoc parsing and messages, formalize chart function arguments.

Also, add a whole lot of extra type checking.

Test Plan

Built and charted various functions with various valid and invalid argument lists, got sensible-seeming errors and results.

Diff Detail

Repository

Branch

chart9

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22623
Build 31008: Run Core Tests
Build 31007: arc lint + arc unit

Event Timeline

epriestley created this revision.Apr 17 2019, 6:29 PM

Herald added a subscriber: yelirekim. · View Herald TranscriptApr 17 2019, 6:29 PM

Harbormaster completed remote builds in B22623: Diff 48753.Apr 17 2019, 6:30 PM

epriestley requested review of this revision.Apr 17 2019, 6:30 PM

Make the "constant" function actually read its argument.

Harbormaster completed remote builds in B22624: Diff 48754.Apr 17 2019, 6:36 PM

This feature is really coming along, practically ready to ship!

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

epriestley added a child revision: D20446: Simplify implementation of "pure" Chart functions.Apr 17 2019, 10:08 PM

amckinley accepted this revision.Apr 18 2019, 11:37 PM

amckinley added inline comments.

src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php
130	"requesting"
src/applications/fact/chart/PhabricatorXChartFunction.php
9	Just for the sake of code coverage, have you tried making any functions accept more than one argument? There's a lot of fancy looping/iterating in this diff that I don't think is getting put through its paces by the existing 0/1-argument functions.

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

In a future diff, scale() and shift() take two arguments and work, although I'm likely rethinking exactly how the composition/chaining part of this works (see D20452 for rambling).

Typo fix.

Harbormaster completed remote builds in B22703: Diff 48828.Apr 23 2019, 7:23 PM

Closed by commit rPedaf17f3fe51: Make chart function argument parsing modular/flexible with 900 pages of error… (authored by epriestley). · Explain WhyApr 23 2019, 7:26 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

__phutil_library_map__.php

applications/

fact/

chart/

PhabricatorChartFunction.php

46 lines

PhabricatorChartFunctionArgument.php

129 lines

PhabricatorChartFunctionArgumentParser.php

154 lines

PhabricatorConstantChartFunction.php

24 lines

PhabricatorFactChartFunction.php

37 lines

PhabricatorSinChartFunction.php

30 lines

PhabricatorXChartFunction.php

9 lines

controller/

PhabricatorFactChartController.php

3 lines

fact/

PhabricatorFact.php

4 lines

Diff 48753

src/__phutil_library_map__.php

Loading...

src/applications/fact/chart/PhabricatorChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorChartFunctionArgument.php

Loading...

src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php

Loading...

src/applications/fact/chart/PhabricatorConstantChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorFactChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorSinChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorXChartFunction.php

Loading...

src/applications/fact/controller/PhabricatorFactChartController.php

Loading...

src/applications/fact/fact/PhabricatorFact.php

Loading...