Removal of the FlutterModuleType, issue #748. #1213
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flutter modules are no longer defined as modules with the FlutterModuleType, instead they are defined as modules with the Dart SDK enabled on the module, where the SDK path is defined inside of a Flutter SDK. See FlutterModuleUtils.isFlutterModule(). Flutter Bazel projects are defined by FlutterModuleUtils.getFlutterBazelWorkspace(), as a project that has a Dart SDK enabled on some module and the module has flutter or module in the directory path. The requirement to have the Dart SDK is a stronger requirement, but is already required for analysis from the Dart Analysis Server, so this will not be a problem. Small IDEs support (aka WebStorm) for project creation and support has not changed. This PR has updated and refactored FlutterModuleUtils to be more of a common interface and definition for determining the Flutterness of some Module or Project. The FlutterInitializer detects the now deprecated module type , "FLUTTER_MODULE_TYPE", and converts these modules into the type of "WEB_MODULE". On the first load of an existing project, users will see a notification from the framework "Unknown Module Type".
|
Thanks! Will look at this today. |
devoncarew
reviewed
Jul 17, 2017
| @@ -211,8 +217,17 @@ public String getParentGroup() { | |||
| } | |||
|
|
|||
| @Override | |||
| @NotNull | |||
| public String getBuilderId() { | |||
| // The builder id is used to distingish between different builders with the same module type, see | |||
| public static void convertFromDeprecatedModuleType(@NotNull Project project) { | ||
| for (Module module : getModules(project)) { | ||
| if (isDeprecatedFlutterModuleType(module)) { | ||
| setFlutterModuleType(module); |
There was a problem hiding this comment.
When we do this, we should show a toast (FlutterMessages.showInfo) letting the user know - it'll help take the edge off the unknown module type toast that will also likely appear.
|
Patching in locally and tested against:
lgtm! The only feedback I have is that we should experiment with a toast - using the flutter icon and some very brief verbiage - to let the user know that their project metadata has been updated, and that the other (error) toast they're seeing from intellij is related and can be safely ignored. |
|
Comments addressed: PTAL at the changes. |
devoncarew
approved these changes
Jul 17, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flutter modules are no longer defined as modules with the FlutterModuleType, instead they are defined as modules with the Dart SDK enabled on the module, where the SDK path is defined inside of a Flutter SDK. See FlutterModuleUtils.isFlutterModule().
Flutter Bazel projects are defined by FlutterModuleUtils.getFlutterBazelWorkspace(), as a project that has a Dart SDK enabled on some module and the module has flutter or module in the directory path. The requirement to have the Dart SDK is a stronger requirement, but is already required for analysis from the Dart Analysis Server, so this will not be a problem.
Small IDEs support (aka WebStorm) for project creation and support has not changed.
This PR has updated and refactored FlutterModuleUtils to be more of a common interface and definition for determining the Flutterness of some Module or Project.
The FlutterInitializer detects the now deprecated module type , "FLUTTER_MODULE_TYPE", and converts these modules into the type of "WEB_MODULE". On the first load of an existing project, users will see a notification from the framework "Unknown Module Type".