Add explicit mysql.port configuration
See: https://github.com/facebook/phabricator/pull/356
Reviewed by: epriestley
Description
Description
Details
Details
- Auditors
edward - Provenance
levijackson Authored on Jul 14 2013, 11:02 PM epriestley Committed on Jul 14 2013, 11:06 PM - Reviewer
- epriestley
- Parents
- rPa0084bbb0d51: Build an early multi-step repository create form
- Branches
- Unknown
- Tags
Event Timeline
Comment Actions
There's an extremely serious indenting bug with this commit that needs fixing. (Other issues inline.)
| /src/applications/config/option/PhabricatorMySQLConfigOptions.php | ||
|---|---|---|
| 72 | Indent is wrong!!!!!!11111 | |
| /src/infrastructure/storage/configuration/DefaultDatabaseConfigurationProvider.php | ||
| 4 | This interface should include getPort() otherwise other custom implementations will fatal: HipHop Fatal error: Call to undefined method FBDatabaseConfigurationProvider::getPort ... | |
Comment Actions
I fixed the indenting and added getPort to the interface:
https://github.com/levijackson/phabricator/commit/eed77d29f595dd13352710d6de35c2998fae5c44
After I rebased and compared my branch to facebook/phabricator I saw that PhabricatorStorageManagementAPI was missing the $port property from:
https://github.com/levijackson/phabricator/commit/04fec8e812d10cbd1985222d61da06e053669987
| /src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php | ||
|---|---|---|
| 6 | This is also missing the $port property | |
| /src/infrastructure/storage/configuration/DefaultDatabaseConfigurationProvider.php | ||
|---|---|---|
| 4 | We just got bit by this; why isn't getPort in the interface if we assume that it exists? | |
Comment Actions
Sorry, I never got CC'd on this since I was only the reviewer. Possibly we should make the audit rules more extensive; D6816 is the first time I saw the issue.