Move IdeClient.connect() to initializeApp().#8282
Conversation
|
Size Change: -307 B (0%) Total Size: 13.2 MB ℹ️ View Unchanged
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
418a861 to
0de7874
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a couple of nice refactorings. First, it moves the IdeClient connection logic from Config.initialize() into initializeApp(). This is a good architectural improvement that ensures the IDE client is ready before the UI starts, and it also improves the separation of concerns between the core and cli packages. Second, it removes the getIdeTrust helper function and its associated file, inlining the logic where it's used. This simplifies the codebase by removing an unnecessary layer of abstraction. The changes are well-implemented and the tests have been updated accordingly. Overall, this is a solid improvement to the codebase.
TLDR
Moves IdeClient initialization out of
config.initialize()and intoinitializeApp().Also, refactor to remove unnecessary file that contained one method used in exactly one other file.
Dive Deeper
IdeClient needs to be initialized before react starts because we might need to show a popup right away.
Reviewer Test Plan
Testing Matrix
Linked issues / bugs
Fixes #8274