-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add support for Android Studio Chipmunk (2021.2), Dolphin (2021.3), Electric Eel (2022.1) and IntelliJ with Java17 runtime #439
Conversation
7ee9f1d
to
14f6378
Compare
4c4cc26
to
aa0187c
Compare
… add support for IntelliJ with Java 17 runtime
aa0187c
to
711a090
Compare
src/main/java/org/gradle/profiler/studio/launcher/LauncherConfigurationParser.java
Show resolved
Hide resolved
...in/src/main/java/org/gradle/profiler/studio/plugin/GradleProfilerProjectManagerListener.java
Show resolved
Hide resolved
...udio-plugin/src/main/java/org/gradle/profiler/studio/plugin/client/GradleProfilerClient.java
Outdated
Show resolved
Hide resolved
...udio-plugin/src/main/java/org/gradle/profiler/studio/plugin/client/GradleProfilerClient.java
Outdated
Show resolved
Hide resolved
// but it doesn't return error message so we rather listen to GRADLE_SYNC_TOPIC to get the sync result | ||
ProjectSystemUtil.getSyncManager(project).syncProject(ProjectSystemSyncManager.SyncReason.USER_REQUEST).get(); | ||
} catch (InterruptedException | ExecutionException e) { | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still call synclistener.syncFailed()
here, or is it okay to just throw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to call sync failed, to not crash our plugin, changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are now back to throwing an exception instead of syncListener.failed()
, is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it seems github is showing outdated version here. Current version is here:
https://github.com/gradle/gradle-profiler/pull/439/files#diff-8eb36c0b3730fb2241bb7c46b8367c29778fcf7971216d69e1fb5df5d9938d1e
...plugin/src/main/java/org/gradle/profiler/studio/plugin/system/AndroidStudioSystemHelper.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/gradle/profiler/studio/plugin/system/GradleProfilerGradleSyncListener.java
Outdated
Show resolved
Hide resolved
...-plugin/src/test/groovy/org/gradle/profiler/studio/plugin/StudioPluginIntegrationTest.groovy
Outdated
Show resolved
Hide resolved
...-plugin/src/test/groovy/org/gradle/profiler/studio/plugin/StudioPluginIntegrationTest.groovy
Outdated
Show resolved
Hide resolved
...-plugin/src/test/groovy/org/gradle/profiler/studio/plugin/StudioPluginIntegrationTest.groovy
Outdated
Show resolved
Hide resolved
cade1ea
to
75efc92
Compare
...plugin/src/main/java/org/gradle/profiler/studio/plugin/system/AndroidStudioSystemHelper.java
Outdated
Show resolved
Hide resolved
...-plugin/src/test/groovy/org/gradle/profiler/studio/plugin/StudioPluginIntegrationTest.groovy
Outdated
Show resolved
Hide resolved
2393822
to
74adecd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM.
and IntelliJ with Java17 runtime.
Fixes #437