Page MenuHomePhabricator

Allow Javelin initBehavior to source alternative library behaviors
ClosedPublic

Authored by ChristopherHJohnson on Nov 4 2014, 12:35 PM.

Details

Summary

Ref T6467. Opens up initBehavior for non-phabricator sourced behaviors.

Test Plan

Confirmed no impact on unset (default 'phabricator' source name) calls to initBehavior

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ChristopherHJohnson retitled this revision from to Allow Javelin initBehavior to source alternative library behaviors.
ChristopherHJohnson updated this object.
ChristopherHJohnson edited the test plan for this revision. (Show Details)
ChristopherHJohnson added a reviewer: epriestley.
epriestley requested changes to this revision.Nov 4 2014, 1:56 PM
epriestley edited edge metadata.
epriestley added inline comments.
src/infrastructure/javelin/Javelin.php
6–8

By convention, indent two spaces.

8

In this codebase, prefer $source_name = 'phabricator' to establish a default scalar parameter value (i.e., an explicit default). This:

  • is clearer for humans when browsing the method signature;
  • is accessible to documentation generators and static analysis;
  • does not generate errors like this one:
[2014-11-04 05:54:04] ERROR 2: Missing argument 3 for Javelin::initBehavior(), called in /INSECURE/devtools/phabricator/src/view/page/menu/PhabricatorMainMenuSearchView.php on line 49 and defined at [/INSECURE/devtools/phabricator/src/infrastructure/javelin/Javelin.php:8]"
This revision now requires changes to proceed.Nov 4 2014, 1:56 PM
epriestley updated this object.Nov 4 2014, 1:57 PM
epriestley edited edge metadata.
ChristopherHJohnson edited edge metadata.

establishs a default scalar parameter value for source_name

ChristopherHJohnson edited edge metadata.

corrects indentation to two spaces lines 6-8

epriestley accepted this revision.Nov 4 2014, 2:46 PM
epriestley edited edge metadata.
This revision is now accepted and ready to land.Nov 4 2014, 2:46 PM
This revision was automatically updated to reflect the committed changes.