Skip to content

[CQ] API: migrate FlutterSdkManager off projectOpened #8123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 28, 2025

Conversation

pq
Copy link
Contributor

@pq pq commented Apr 28, 2025

Move FlutterSdkManager (unstable) post-project open initialization to a (supported) ProjectActivity.

See: #8100, #6953


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

@pq pq requested a review from jwren April 28, 2025 19:58
@jwren
Copy link
Member

jwren commented Apr 28, 2025

This resolves the single call to projectOpened, but now we have the functionality split between two files, FlutterSdkManager and FlutterSdkManagerStartupActivity.kt, wouldn't it be more prudent to migrate the entire file over so that 1) we have one file where this logic is all shared, and 2) we don't increase the number of entries in the plugin.xml files?

With the bootstrappers, the loading in of the extensions isn't always consistent. This will potentially change the order of operations for all users or some users.

@pq
Copy link
Contributor Author

pq commented Apr 28, 2025

Good questions!

I don't think we want (all of) the FlutterSdkManager registered as a startup activity and am inclined to think that checkForFlutterSdkChange should never have been in it the first place but I'm happy to discuss. (I'm not even sure it can be since it's primarily a project service.)

As for duplicate entries, even if the class were shared, it would need two entries in the xml for projectService and backgroundPostStartupActivity registrations so I don't see a way to reduce that. Or maybe you meant something else?

One thought is that we could make this a more general FlutterSdkStartupActivity so that other post startup activity logic could be hosted here too. (Another reason not to make FlutterSdkManager a ProjectActivity.)

Happy to do that when I tackle the next projectOpened migration if it sounds good to you.

@pq pq merged commit ec2ec2b into flutter:master Apr 28, 2025
7 checks passed
@pq pq deleted the cq_FlutterSdkManager_postStartup branch April 28, 2025 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants