Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7977: Associate Symbols with Repositories directly (Instead of arc projects)
T7220: Repository without Arcanist project fails for symbols import - Commits
- Restricted Diffusion Commit
rP2483f6f120a4: Move symbols to be repository-based
- Added symbols and then applied migrations, symbols seemed to be migrated successfully.
- Tested the /diffusion/symbol/$SYMBOL_NAME endpoint.
- Tested the /diffusion/symbol/$SYMBOL_NAME endpoint with the ?repositories=$REPOSITORY_PHID parameter.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Couple of notes.
| resources/sql/autopatches/20150429.repositorysymbols.sql | ||
|---|---|---|
| 3 ↗ | (On Diff #30272) | Although I don't think this feature is too heavily used, it would be nice to try to migrate, and doesn't seem too difficult:
|
| scripts/symbols/import_project_symbols.php | ||
| 0 | This file should be renamed. | |
| 0 | We could probably just hard code this to like 4K or 8K or something reasonable-ish. | |
| src/applications/diffusion/controller/DiffusionSymbolController.php | ||
| 33–40 | This should use RepositoryQuery and be policy-aware. | |
| 106 | Consider getMonogram() in new code to prepare for making callsigns optional eventually. | |
| src/applications/diffusion/query/DiffusionSymbolQuery.php | ||
| 74–75 | Maybe modernize to with... while the method name is changing anyway. | |
| 251 | Should be RepositoryQuery | |
I'm currently getting Bad getter call: getRepositoryPHID when trying to hit DiffusionSymbolController.
| src/applications/repository/storage/PhabricatorRepositorySymbol.php | ||
|---|---|---|
| 10–12 | Maybe because this class removes arcanistProjectID but doesn't add protected $repositoryPHID? | |
| src/applications/repository/storage/PhabricatorRepositorySymbol.php | ||
|---|---|---|
| 10–12 | Ah thanks. | |
This also fixes T7977. Caught some minor junk inline.
| resources/sql/autopatches/20150429.repositorysymbols.2.php | ||
|---|---|---|
| 15 ↗ | (On Diff #30356) | Ignoring it and continuing is fine/reasonable/correct as far as I'm concerned. |
| scripts/symbols/clear_repository_symbols.php | ||
| 29 | $more undefined, duplicates intent above? | |
| 30 | Use RepositoryQuery. | |
| 34–41 | Duplicate code. | |
Looks like we started on opposite ends, which is good.
My version adds (diff):
- Move "Use Symbols From" and "Languages" to repository object; Updated the JS and DiffusionBrowseFileController accordingly
I can rebase off this, and submit as a separate revision.
| resources/sql/autopatches/20150429.repositorysymbols.2.php | ||
|---|---|---|
| 15 ↗ | (On Diff #30356) | I'm not even sure how this works exactly, because the repositoryPHID field is marked NOT NULL. |
| resources/sql/autopatches/20150429.repositorysymbols.2.php | ||
|---|---|---|
| 15 ↗ | (On Diff #30356) | It will default to the empty string if it's marked NOT NULL (and has a VARCHAR, VARBINARY, TEXT, etc., type). |
I just tested ./bin/remove destroy and it seems that I broke it.
Destroying PhabricatorRepository rARC...
[2015-05-03 11:36:05] EXCEPTION: (AphrontSchemaQueryException) #1054: Unknown column 'id' in 'where clause' at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:305]
arcanist(head=master, ref.master=977baacc329c), phabricator(head=master, ref.master=a8c29736fb32), phutil(head=master, ref.master=00e1f2da2829)
#0 AphrontBaseMySQLDatabaseConnection::throwQueryCodeException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:275]
#1 AphrontBaseMySQLDatabaseConnection::throwQueryException(mysqli) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:181]
#2 AphrontBaseMySQLDatabaseConnection::executeRawQuery(string) called at [<phutil>/src/xsprintf/queryfx.php:6]
#3 queryfx(AphrontMySQLiDatabaseConnection, string, string, string, NULL)
#4 call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:26]
#5 AphrontDatabaseConnection::query(string, string, string, NULL) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1176]
#6 LiskDAO::delete() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:1172]
#7 PhabricatorRepository::delete() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:1876]
#8 PhabricatorRepository::destroyObjectPermanently(PhabricatorDestructionEngine) called at [<phabricator>/src/applications/system/engine/PhabricatorDestructionEngine.php:40]
#9 PhabricatorDestructionEngine::destroyObject(PhabricatorRepository) called at [<phabricator>/src/applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php:98]
#10 PhabricatorSystemRemoveDestroyWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:396]
#11 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:292]
#12 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/setup/manage_remove.php:21]
PHP Fatal error: Uncaught exception 'Exception' with message 'Process exited with an open transaction! The transaction will be implicitly rolled back. Calls to openTransaction() must always be paired with a call to saveTransaction() or killTransaction().' in /usr/src/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:69
Stack trace:
#0 [internal function]: AphrontDatabaseTransactionState->__destruct()
#1 {main}
thrown in /usr/src/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php on line 69
Fatal error: Uncaught exception 'Exception' with message 'Process exited with an open transaction! The transaction will be implicitly rolled back. Calls to openTransaction() must always be paired with a call to saveTransaction() or killTransaction().' in /usr/src/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:69
Stack trace:
#0 [internal function]: AphrontDatabaseTransactionState->__destruct()
#1 {main}
thrown in /usr/src/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php on line 69