Skip to content
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

Define proto module system #1819

Merged
merged 4 commits into from
Feb 27, 2018
Merged

Define proto module system #1819

merged 4 commits into from
Feb 27, 2018

Conversation

stevemessick
Copy link
Member

@stevemessick stevemessick commented Feb 23, 2018

@devoncarew @pq

Define a project system for Flutter. Fixes compilation to build 3.1 and 3.0 from common sources.

Changes the build command to cause incompatible files to be ignored during 3.0 build. Makes a template for studio-contribs.xml.

Fixes #1824.

@stevemessick
Copy link
Member Author

As expected, this does not compile against AS 3.0.

The merge conflict will be easy to resolve, once we get to the point that it needs to be done.

@devoncarew devoncarew mentioned this pull request Feb 24, 2018
7 tasks
@stevemessick
Copy link
Member Author

PTAL

@stevemessick
Copy link
Member Author

I can resolve the conflict during the merge, if there are no other changes that need to be made.

@devoncarew @pq

@devoncarew
Copy link
Member

(looking now)

@@ -34,5 +34,8 @@
<orderEntry type="module" module-name="gradle" scope="TEST" />
<orderEntry type="library" scope="TEST" name="truth" level="project" />
<orderEntry type="module" module-name="flutter-intellij-community" />
<orderEntry type="module" module-name="project-system" />
<orderEntry type="module" module-name="project-system-gradle" />
<orderEntry type="module" module-name="android-ndk" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious - these were added automatically by IntelliJ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. They are all required by new code.

@@ -25,7 +25,7 @@ public void run() {
ApplicationInfo info = ApplicationInfo.getInstance();
if ("Google".equals(info.getCompanyName())) {
String version = info.getFullVersion();
if (version.startsWith("2.") || (version.contains("Beta") && !version.endsWith("7"))) {
if (version.startsWith("2.")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This likely address #1824 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I updated the intro to make that clear.

@Nullable
@Override
public VirtualFile getDefaultApkFile() {
return gradleProjectSystem.getDefaultApkFile(); // TODO find the flutter binary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this todo:? If so, can you provide more context (for whoever comes through here net)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@@ -7,7 +7,11 @@
"ideaVersion": "171.4408382",
"dartPluginVersion": "171.4424.10",
"sinceBuild": "171.1",
"untilBuild": "171.*"
"untilBuild": "171.*",
"filesToSkip": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,38 @@
<!-- Defines Android Studio IDE-specific contributions and implementations. -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to look into a system to generate plugin.xml and studio-contribs.xml from the templates and remove the others from source control. Or, have a task on travis fail if changes are made to one file but not to the the other (template and non-template), else they risk getting out of sync.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this sounds good, lets open an issue to track.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to get into the habit of using plugin gen.

Adding a fail-safe check to travis sounds good, too. See #1842.

result = await runner.javac2(spec);
for (var file in spec.filesToSkip) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this block may want to be in a finally block, so that it's performed even if there are exceptions out of the compile stage

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@stevemessick stevemessick merged commit 55ca041 into master Feb 27, 2018
@stevemessick stevemessick deleted the project-system branch February 27, 2018 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants