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
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 5586 Build 5605: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Couple of notes.
resources/sql/autopatches/20150429.repositorysymbols.sql | ||
---|---|---|
3 | 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 | ||
10 | This file should be renamed. | |
30–37 | We could probably just hard code this to like 4K or 8K or something reasonable-ish. | |
src/applications/diffusion/controller/DiffusionSymbolController.php | ||
33 | This should use RepositoryQuery and be policy-aware. | |
104 | Consider getMonogram() in new code to prepare for making callsigns optional eventually. | |
src/applications/diffusion/query/DiffusionSymbolQuery.php | ||
74 | Maybe modernize to with... while the method name is changing anyway. | |
246 | Should be RepositoryQuery |
I'm currently getting Bad getter call: getRepositoryPHID when trying to hit DiffusionSymbolController.
src/applications/repository/storage/PhabricatorRepositorySymbol.php | ||
---|---|---|
10–11 | Maybe because this class removes arcanistProjectID but doesn't add protected $repositoryPHID? |
src/applications/repository/storage/PhabricatorRepositorySymbol.php | ||
---|---|---|
10–11 | 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 | ||
28 ↗ | (On Diff #30356) | $more undefined, duplicates intent above? |
29 ↗ | (On Diff #30356) | Use RepositoryQuery. |
33–40 ↗ | (On Diff #30356) | 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