Summary:
Phabricator generates a bunch of data that we don't need to keep around forever,
add a GC daemon to get rid of it with some basic configuration options.
This needs a couple more diffs to get some of the details but I think this is a
reasonable start.
I also fixed a couple of UI things related to this, e.g. the daemon logs page
going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout.
Test Plan:
- Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once
it had cleaned everything up.
- Mucked around with TTLs and verified they work correctly.
- Viewed gc'd transcripts in the web interface and made sure they displayed
okay.
- Viewed daemon logs before/after garbage collection.
- Running some run-at / run-for tests now, I'll update if the daemon doesn't
shut off in ~10-15 minutes. :P
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 583
Summary: When the remote closes the connection, phabot goes into a busy loop
because of PHP's "nothing should ever be an error" semantics. Instead, detect
connection termination.
Test Plan: Disabled the "PONG" response in the protocol handler and let freenode
disconnect phabot. It spun out of control before, now it detects the issue and
exits to await automatic restart.
Reviewed By: jungejason
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 582
Summary:
Summarize (or at least link to) pastes when P<id> is given in a channel. This depends on D552!
Test Plan:
04:34:03 <@codeblock-phabot> P15 - http://phabricator.local/P15
04:35:22 <@CodeBlock> P1 asdf foo Bar Baz Diet Sprite P4
04:35:24 <@codeblock-phabot> P1 - http://phabricator.local/P1
04:35:24 <@codeblock-phabot> P4 - http://phabricator.local/P4
Reviewers:
epriestley
CC:
Differential Revision: 553
Summary:
Maniphest tasks can now be summarized by referencing their respective T<id> in irc.
Test Plan:
03:29:28 <@CodeBlock> T218
03:29:29 <@codeblock-phabot> T218: NYANCAT (Priority: Wishlist) - https://secure.phabricator.com/T218
03:30:14 <@CodeBlock> T58 asdf foo bar baz mcFoo T163
03:30:15 <@codeblock-phabot> T58: Allow AphrontFormView to render as a <div /> instead of a form (Priority: Wishlist) - https://secure.phabricator.com/T58
03:30:15 <@codeblock-phabot> T163: arc amend is slow (Priority: Needs Triage) - https://secure.phabricator.com/T163
...
03:28:23 <@CodeBlock> Oh. Hah, I'm testing against production Conduit. Ah well.
03:28:40 <@CodeBlock> I don't always test my code...but when I do, I do it in prod! :D
Reviewers:
epriestley
CC:
Differential Revision: 551
Summary:
When a user gets @mentioned in Differential, add them as a CC.
No Maniphest hookup yet since I want to make that one a little more formal.
Depends on D518.
Test Plan:
@mentioned a user and they were added as a CC.
Reviewed By: jungejason
Reviewers: tomo, mroch, jsp, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 519
Summary:Make a new directory, src/infrastructure/markup/remarkup/markuprule/paste/
Make a new class called PhabricatorRemarkupRulePaste in that directory.
Add the rule to DifferentialMarkupEngineFactory.
Test Plan: Created a task in maniphest. Put P1 and P2 in the content.
Created P1 and P2 in Paste. Verified P1 and P2 were highlighted and
linked correctly.
Reviewers:epriestley, codeblock
CC:jungejason
Differential Revision: 539
Summary:
The web console already correctly checks for processes which are running but
can't be signaled. Share this check in the CLI.
Test Plan:
Looked at the web console. Poked at the CLI.
Reviewed By: fratrik
Reviewers: fratrik
CC: aran, fratrik
Differential Revision: 544
Summary:
Currently we send logs to the logfile in 'phd debug', but we should send them to
the console instead.
Also fixed some %C stuff which could theoretically cause problems if a user had
percentage symbols in their paths (heaven forbid).
fratrik, this or D535 might have been involved in frustrating your efforts to
debug the "sudo" stuff.
Test Plan:
Ran "phd debug irc derpderp" and "phd launch irc derpderp". In the former case,
the exception appeared in the console. In the latter, it appeared in the log.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran, fratrik
CC: aran, codeblock
Differential Revision: 536
Summary:
Provides basic Remarkup support for @mentions. No application integration yet so
these aren't terribly useful until that happens.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-83d68e7af6085ae928df/
Reviewers: tomo, mroch, jsp
Commenters: tomo
CC: aran, tomo, epriestley
Differential Revision: 517
Summary:
The MySQL MyISAM fulltext backend defaults to a minimum length of 4, but many
3-letter words are useful search terms. Provide instructions for configuring
MySQL to reduce the auto-stopword length.
Test Plan:
Followed instructions, searched for a 3-letter term and got a hit.
Reviewed By: jungejason
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, jungejason
Differential Revision: 471
Summary:
Tweaks to the paste app:
- I realized that unlike all the other apps, it makes more sense for the
default view of this one to be "create paste" instead of "list pastes" since
when you access the application directly you are most often wanting to share
something. Swap list out of the default slot and make edit the default.
- Make the textarea bigger (usability).
- Allow you to copy an existing paste.
- Implement 'raw view'.
- Tweak/adjust list view (usability, formatting).
- Tweak page titles.
Test Plan:
Created, copied, and listed pastes. Viewed raw paste. Created an invalid paste.
Tried to create a copy of a nonexistant paste.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, epriestley, codeblock
Differential Revision: 456
Summary:
Nicks can contain more characters than were allowed. The new regex is ugly,
but should include most of them.
Test Plan:
Joined the bot as phabot-c0d`eb1^ock successfully.
Reviewers:
epriestley
CC:
Differential Revision: 453
Summary:
remove accessing the db config info directly. Use
DatabaseConfigurationProvider instead. Also fixed a minor issue where
different number of newlines are output in PhabricatorSetup.php's output.
Test Plan:
executed upgrade_schema.php; executed PhabricatorSetup.php by
setting 'phabricator.setup' to true.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, jungejason, epriestley
Differential Revision: 443
Summary:
Make 'gd' an explicit optional dependency, test for it in setup, and make the
software behave correctly if it is not available.
When generating file thumnails, provide reasonable defaults and behavior for
non-image files.
Test Plan:
Uploaded text files, pdf files, etc., and got real thumbnails instead of a
broken image.
Simulated setup and gd failures and walked through setup process and image
fallback for thumbnails.
Reviewed By: aran
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 446
Summary:
Make it easier to find obvious problems in daemons by letting them run
undaemonized in the console without requiring the user to know the magical
incantations for loading libraries, etc.
Test Plan:
Ran "phd debug nice", simulated some failures (e.g., bringing down Phabricator,
daemon fatal) and got useful error messages.
Reviewed By: jungejason
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 448
Summary:
- Make the instructional text generally more useful.
- Show the current configured adapter.
- When the configuration prevents outbound email from being delivered, show a
warning.
- Detect 'curl' extension during setup since it's more-or-less required
- Add curl extension to the install scripts
codeblock: can you verify the rhel-derivs changes are correct?
Test Plan:
Set adapter to test, verified warning; entered setup mode and verified curl. Ran
apt-get on an ubuntu box. Ran yum on an amazon linux box.
Reviewed By: toulouse
Reviewers: toulouse, codeblock
Commenters: codeblock
CC: aran, jungejason, tuomaspelkonen, codeblock, epriestley, toulouse
Differential Revision: 438
error message if that doesn't work
Summary:
This workflow is needlessly bad right now, make it not terrible.
Also removed some related, unused code.
Test Plan:
Ran 'phd launch nice' with no directory and with a failing mkdir command.
Reviewed By: toulouse
Reviewers: hsb, toulouse, codeblock
CC: aran, toulouse
Differential Revision: 440
Summary:
There was a last-minute edit to this to fix a typo before rP089d8327 landed
which accidentally made it impossible to pass the check. :)
Test Plan:
Put install into setup mode, changed protocol to 'http', 'ftp'.
Reviewed By: cadamo
Reviewers: toulouse, codeblock, cadamo
Commenters: toulouse, codeblock
CC: aran, cadamo, toulouse, codeblock
Differential Revision: 434
Summary:
When a user hits 'cancel' on a 'new', 'edit', or 'reply' operation, add a little
"Changes discarded. __Undo__" insert so they can get their change back. No undo
for delete since there's an explicit prompt. Once this lands we can make
'escape' work again to close dialogs.
This change started feeling really good when I was merging all the duplicate
code and making things more consistent, but by the time I started writing client
rendering it felt gross. I'm not really thrilled with it but I guess it's a step
forward? The feature seems pretty OK in practice. Let me know how much barfing
this causes and I can try to remedy the most acute concerns.
This also fixes a bug where replies always (?) appear on the 'new' side of the
diff (I think?).
Test Plan:
Applied 'new', 'edit', 'delete' and 'reply' operations, pressed 'cancel' and
'okay' in each case, with and without changing text where relevant. All
behaviors seem to conform with expectations, except that canceling out of 'edit'
without changing the text gives you an option to undo when it shouldn't really.
There's no super easy way to get at the original text right now.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: simpkins, aran, epriestley
Differential Revision: 406
Summary:
Well, since I couldn't regenerate my arcanist cert I figured out that this wass because "workflows" are unavailable there now. I really can not figure out why but it was.
I added in the setup script, the ability to check if is present the protocol of the host and if it has a trailing slash a the end of the line, since both are needed to generate the cert.
Users now only be able to upload valid image files with mimetype of jpg, jpeg,
png and gif.
Test Plan:
FIRST: DO NOT apply those changes! then
1- go to settings->arcanist certificate and the click on regenerate ... humm
2- On your config file, delete the trailing slash at the end and the protocol on "phabricator.base-uri", then go to setting->arcanist certificate. Here you
will see something like this "phabricator.example.comapi\/" instead of
"http:\/\/phabricator.example.com\/api\/".
SECOND: Now apply this changes:
1- Go to settings->arcanist certificate and the click on regenerate.
2- On your config file, delete the trailing slash at the end and the protocol
on "phabricator.base-uri", and setup "phabricator.setup" to true.
3- Then go to setting->arcanist certificate and you could see that this was successfully generated.
THIRD:
Go to settings->account and try to upload an invalid image file, and do the same on "youruserna"->edit profile.
Reviewed By: epriestley
Reviewers: epriestley jungejason
CC: epriestley jugesason cadamo aran
Differential Revision: 391
Summary:
Allows you to link to comments with "D123#3" or "T123#3", then adds a pile of JS
to try to make it not terrible. :/
The thing I'm trying to avoid here is when someone says "look at this!
http://blog.com/#comment-239291" and you click and your browser jumps somewhere
random and you have no idea which comment they meant. Since I really hate this,
I've tried to avoid it by making sure the comment is always highlighted.
Test Plan:
Put T1#1 and D1#1 in remarkup and verified they linked properly.
Clicked anchors on individual comments.
Faked all comments hidden in Differential and verified they expanded on anchor
or anchor change.
Reviewed By: aran
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 383
Summary:
In RHEL6 at the least, pcntl installs from distro package management to the CLI
but not to Apache. Since we don't need it in apache and it's a pain to build
manually, just verify it exists on the CLI.
Test Plan:
Simulated script failures and verified setup output.
Reviewed By: codeblock
Reviewers: codeblock, aran, jungejason, tuomaspelkonen
CC: aran, epriestley, kevinwallace, codeblock
Differential Revision: 380
Summary:
Sendmail is seriously difficult to configure; SendGrid is extremely easy. It's
also pretty expensive ($80/mo) but there are a bunch of startups that already
have plans so it's effectively free for them.
Test Plan:
Configured SendGrid and sent reply email through it.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 376
Summary:
Keep him from getting killed every 24 hours by the overseer, add basic commit
support.
Test Plan:
Ran irc bot, fed him a commit, fed him "http://blah/D1".
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, codeblock, mroch
CC: aran, epriestley
Differential Revision: 377
Summary:
Removes calling `which` and instead checks the file path that PHPMailerLite uses directly (/usr/bin/sendmail).
This fixes $PATH export issues which occur on certain platforms causing un-expected errors during setup.
Test Plan:
* Run setup on server without sendmail install & error should be presented.
* Install sendmail then re-run setup & no error should be presented.
Reviewers: epriestley
Differential Revision: 375
Summary:
After successfully installing phabricator on my Mac OS X 10.6.7, I was unable to
link my accounts to either Facebook or GitHub.
I diagnosed that file_get_contents() and fopen() were not working properly.
After installing the php openssl package I was able to get it linking
successfully.
Test Plan:
With php's openssl extension disabled, and phabricator installed. Try linking to
Facebook and GitHub and observe that it fails. You can visit the Auth
Diagnostics page and "Facebook Graph" and "App Login" should fail.
With php's openssl extension enabled, linking to Facebook and GitHub should be
successful.
Change the configuration to add "phabricator.setup = false".
Disable php's openssl extension. Visit the phabricator site and observe that it
requires you to install php's openssl extension.
Enable php's openssl extension. Visit the phabricator site and observe that it
installs fine.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 352
Summary:
I need to add some more conduit methods / info to make this at all useful but
here's some basics.
Test Plan:
Hung out in #phabot-test
Reviewed By: mroch
Reviewers: mroch, codeblock, aran, jungejason, tuomaspelkonen
CC: aran, mroch, epriestley
Differential Revision: 300
Summary:
Allow construction of handlers which use Conduit.
Test Plan:
Made a bot that connects to local and runs conduit.ping.
Reviewed By: mroch
Reviewers: mroch, codeblock, aran, jungejason, tuomaspelkonen
CC: aran, mroch
Differential Revision: 299
Summary:
This is purely a prototype at the moment, but the basic functionality sort of
works.
I'm not sure how far I want to go with this but I think we might be able to get
somewhere without it being gross.
The idea here is to build a notification server WITHOUT using Comet, since Comet
is extremely difficult and complicated.
Instead, I use Flash on the client. LocalConnection allows flash instances to
talk to each other and connect() can be used as a locking primitive. This allows
all the instances to elect a master instance in a race-safe way. The master is
responsible for opening a single connnection to the server.
On the server, I use Node.js since PHP is pretty unsuitable for this task.
See Github Issue #3: https://github.com/facebook/phabricator/issues/3
One thing I need to figure out next is if I can reasonably do SSL/TSL over Flash
(it looks like I can, in theory, with the as3crypto library) or if the server
needs to just send down version information and trigger a separate Ajax call on
the client.
Test Plan:
Created a client pool and connected it to the server, with election and failover
apparently working correctly.
Reviewed By: aran
Reviewers: Girish, aran, jungejason, tuomaspelkonen, davidrecordon
Commenters: Girish, davidrecordon
CC: aran, epriestley, Girish, davidrecordon
Differential Revision: 284
Summary:
Get rid of the Phabricator-level DarkConsole-specific API and use the more
general Phutil-level one.
Test Plan:
Loaded DarkConsole services plugin, viewed Diffusion, got execs in the trace.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran
Differential Revision: 293
Summary:
This is sort of a silly/fun project but I think there's some utility. For
example, mroch added some handlers to an eggdrop or something similar to look
for "D12345" and print out the title/link, which was actually pretty useful.
We could also add logging here and subsume the more-or-less unowned Facebook
tool that does the same thing, especially since we can get a bunch of good stuff
it doesn't support (like search) more or less for free.
This is also an easy way to provide some example code for writing Conduit system
agents.
This is a minimal implementation which creates a bot that connects to a
hard-coded server and sits there indefinitely. Next steps:
- Add conduit/sysagent support
- Write differential/maniphest/diffusion handlers
- Move configuration to the web interface (?) and integrate with phd
- Write a logging handler?
Test Plan:
Ran bot with "exec_daemon.php", it connected to the hard-coded server and sat
there indefinitely.
Reviewed By: aran
Reviewers: codeblock, mroch, tuomaspelkonen, aran, jungejason
CC: aran, epriestley
Differential Revision: 283
Summary:
ccheever did an install and gave me some feedback about issues he hit. This
tries to:
- properly document how to configure outbound email;
- test outbound email configuration in the setup mode;
- provide basic daemon documentation;
- document that phabricator.base-uri is required for all installs.
Test Plan:
read documentation, jumped through all the setup branches to test configuration
error detection
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran, rm
CC: ccheever, aran
Differential Revision: 276
Summary:
Currently, the Javelin linter fails completley if this binary is missing.
However, it's hard to build and not critical so just issue a warning.
Eventually we can document this better and make the build easier, but the
current behavior is pretty unfriendly so make it smoother until the state of the
world can be improved.
Test Plan:
Removed the binary and ran "arc lint --lintall" against multiple Javelin paths.
Received one warning. Restored the binary and ran with "--trace", got no
warnings and verified that the binary was running.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran, tomo
CC: aran, jungejason
Differential Revision: 265
Summary:
Alters the installation instructions to guide installers into a "setup" mode
which does config file sanity checking.
Test Plan:
Put myself in setup mode, simulated all the failures it detects, took myself out
of setup mode, Phabricator works OK.
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 230
Summary:
We always return HTTP 200 right now and don't send a "Last-Modified" header, so
browsers download more data then necessary if you sit on a page mashing reload
(for example).
Test Plan:
Used Charles to verify HTTP response codes from 400, 404 and 304 responses.
Mashed reload a bunch and saw that the server sent back 304s.
Changed the resource hash seed and saw 200s, then 304s on reload.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: bmaurer, aran, tuomaspelkonen
Differential Revision: 253
Summary:
Javelin is currently embedded in Phabricator via copy-and-paste of prebuilt
packages. This is not so great.
Pull it in as a submodule instead and make all the Phabriator resources declare
proper dependency trees. Add Javelin linting.
Test Plan:
I tried to run through pretty much all the JS functionality on the site. This is
still a high-risk change, but I did a pretty thorough test
Differential: inline comments, revealing diffs, list tokenizers, comment
preview, editing/deleting comments, add review action.
Maniphest: list tokenizer, comment actions
Herald: rule editing, tokenizers, add/remove rows
Reviewed By: tomo
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, tomo, epriestley
Differential Revision: 223
Summary:
Previously, Remarkup allowed you to paste in an image URI and get an inline
image. However, it did this by hotlinking the image which isn't so hot in an
open source product.
Restore this feature, but use image proxying instead. The existing image macro
code does most of the work.
There is a mild security risk depending on the network setup so I've left this
default-disabled and made a note about it. It should be safe to enable for
Facebook.
Test Plan:
Pasted in image and non-image links, got reasonable behavior. Verified proxying
appears to work. Verified that file:// shenanigans produce 400.
Reviewed By: tuomaspelkonen
Reviewers: aran, jungejason, tuomaspelkonen
Commenters: cpiro
CC: aran, cpiro, tuomaspelkonen
Differential Revision: 214
Summary:
Allow Lisk to be put into process-isolated mode which establishes
only isolated connections. By default, put it into this mode when running
unit tests. Build some simple unit tests around object insertion and
updating.
NOTE: The one flaw in this is that $dao->establishConnection() still
punches through the isolation layer. I need to do an API change to fix this
though so I'm holding it for now. It will probably just rename getConnection()
to establishConnection() and then rename establishConnection() to something
scary like establishLiveExternalConnection().
Test Plan:
Ran unit tests.
Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, epriestley
Differential Revision: 194
Summary:
This provides a new connection which doesn't connect to
anything, so effects can be isolated to the current process (for
unit testing).
Test Plan:
Ran unit tests.
Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, epriestley
Differential Revision: 193
Summary:
The correct name of this key is 'github.application-secret', not
'github.secret'. Make DarkConsole check that all the masked keys exist to
prevent this from happening again. This isn't super important since this
is just intended to protected against casual security lapses (taking a
screenshot with DarkCnosole's "Config" tab open, for instance) but it's easy
to check for so it seems worthwhile to get right.
Test Plan:
Loaded page without the actual config file change, got an exception.
Fixed the config, reloaded the page, good news goats (really trying to get this
to catch on since goats are adorable).
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran
Differential Revision: 189
Summary:
Just minor bookkeeping, but the current regexp is too liberal and
will match things which can't possibly be revision hashes.
Test Plan:
Typed things which should and shouldn't be revision links, they
got handled properly.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 185
Summary:
Fixed the image macro regex not to use '-' as the separator.
Also minor improvement to randomon.
Test Plan:
Tried different image marcors.
Reviewed By: jungejason
Reviewers: jungejason
CC: epriestley, jungejason
Differential Revision: 153
Summary: add --load-phutil-library for libraries other than phabricator
and libphutil.
Test Plan: launch a daemon in facebook such as
PhabricatorDifferentialCommentDaemon.
Reviewers: epriestley, tuomaspelkonen
CC:
Differential Revision: 146
Summary: See D133. Workers can also be subject to the same race, invert the
row relationship in the same way.
Test Plan: Launched repository master daemons and some taskmasters and used
the Daemon console to veify that they were able to process tasks. Manually
checked the database to make sure data got linked correctly and that new data
was inserted correctly.
Reviewers: jungejason
CC: tuomaspelkonen
Differential Revision: 135
Summary: While I should fix the transactional stuff, that patch is going to be
tricky and transactions have some performance implications. This is a simple
fix which prevents the race.
Instead of having the data point at the event ID, have the event point at a
data ID. Insert the data first, then insert the event with the right data
pointer. This is super simple and prevents the race issue.
Test Plan:
- Ran the schema upgrade script, verified that the database was
correctly upgraded. Was also prompted to stop daemons.
- Ran 'repository-launch-master', verified that the discovery daemons were
able to discover new commits and insert events for them. Verified the
committask daemon was consuming events and converting them into tasks.
- Verified new tasks looked correct in the database.
- Browsed web interface.
Reviewers: jungejason
CC: tuomaspelkonen
Differential Revision: 133
Summary:
Added long waited image macro support for differential and others.
Test Plan:
Tried a couple of different macros and made sure they appear nicely
in the comment preview. Made sure that the normal comments are shown
correctly.
Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, tuomaspelkonen, epriestley
Differential Revision: 129
Summary: Basic scaffolding for repository tracking, plus daemon infrastructure
(Timelines, Cursors) and some fixes (memory usage, mysql_connect() junk).
Test Plan: parsed Javelin git commit history via daemon
Reviewers:
CC:
Summary: Autolink Differential and Maniphest objects.
Test Plan: Typed "D12345" and "T12345" into the Differential comment preview,
got links. Typed "http://www.elsewhere.com/D12345" and got a single link to
that URI, not a mess where the D12345 part linked incorrectly.
Reviewers: aran
CC:
Differential Revision: 35
