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

Avoid unnecessary task configuration #1111

Merged
merged 9 commits into from
Sep 27, 2022
Merged

Conversation

3flex
Copy link
Contributor

@3flex 3flex commented Sep 21, 2022

Pull Request Details

I noticed in a project that uses this plugin that many tasks were created during the configuration phase which seemed unnecessary.

Description

I have applied various techniques to avoid unnecessary task configuration. As written here https://docs.gradle.org/current/userguide/task_configuration_avoidance.html:

The configuration avoidance API avoids configuring tasks if they will not be needed during the course of a build, which can have a significant impact on total configuration time.

Related Issue

#1110

Motivation and Context

Improves Gradle's performance.

How Has This Been Tested

Applying gradle-intellij-plugin using Gradle's composite build feature in the cashapp/sqldelight project which uses this plugin.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have included my change in the CHANGELOG.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@YannCebron YannCebron requested a review from hsz September 21, 2022 08:13
@hsz hsz added this to the next milestone Sep 21, 2022
This method will cause all tasks in this container to be realized, which on
project.tasks means every task in the project will be configured when the
plugin is applied.
Using forEach on a TaskCollection will eagerly realise those tasks. Using
Gradle's configureEach will avoid realising the tasks unless they will run
as part of the build.
@3flex
Copy link
Contributor Author

3flex commented Sep 23, 2022

@hsz I've rebased on master which now passes CI, and the test that was failing after your last commit succeeds locally, so I'm hopeful this is OK to go though the workflow won't run without a maintainer approval given I'm a first-time contributor.

Happy to address any issues, just let me know how best to help, thanks!

@hsz
Copy link
Member

hsz commented Sep 23, 2022

@3flex Thanks a lot for your PR – an outstanding finding!
Your change is correct, but I must adjust a few other places – mainly integration tests. I'll push all the changes to your branch and merge whenever everything goes green.

karollewandowski and others added 5 commits September 27, 2022 15:52
… execution (JetBrains#1108)

* Do not move resource directories to the end of classpath (JetBrains#1101)

* Do not move resource directories to the end of classpath (JetBrains#1101)

- add integration test

* Do not move resource directories to the end of classpath (JetBrains#1101)

- include in CHANGES.md

* Do not move resource directories to the end of classpath (JetBrains#1101)

- cosmetics

* Bump jackson-databind from 2.13.3 to 2.13.4

Bumps [jackson-databind](https://github.com/FasterXML/jackson) from 2.13.3 to 2.13.4.
- [Release notes](https://github.com/FasterXML/jackson/releases)
- [Commits](https://github.com/FasterXML/jackson/commits)

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.core:jackson-databind
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump structure-intellij from 3.236 to 3.237

Bumps [structure-intellij](https://github.com/JetBrains/intellij-plugin-verifier) from 3.236 to 3.237.
- [Release notes](https://github.com/JetBrains/intellij-plugin-verifier/releases)
- [Commits](JetBrains/intellij-plugin-verifier@intellij-structure-3.236...intellij-structure-3.237)

---
updated-dependencies:
- dependency-name: org.jetbrains.intellij.plugins:structure-intellij
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump structure-base from 3.236 to 3.237

Bumps [structure-base](https://github.com/JetBrains/intellij-plugin-verifier) from 3.236 to 3.237.
- [Release notes](https://github.com/JetBrains/intellij-plugin-verifier/releases)
- [Commits](JetBrains/intellij-plugin-verifier@intellij-structure-3.236...intellij-structure-3.237)

---
updated-dependencies:
- dependency-name: org.jetbrains.intellij.plugins:structure-base
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump dd-plist from 1.24 to 1.25

Bumps [dd-plist](https://github.com/3breadt/dd-plist) from 1.24 to 1.25.
- [Release notes](https://github.com/3breadt/dd-plist/releases)
- [Commits](3breadt/dd-plist@v1.24.0...v1.25.0)

---
updated-dependencies:
- dependency-name: com.googlecode.plist:dd-plist
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Integration Tests: Add missing entries to README file

* Invalidate instrumented classes bound to forms if GUI changed [IDEA-298989](https://youtrack.jetbrains.com/issue/IDEA-298989/Duplicate-method-name-getFont)

* Integration Tests: extend classpath test with JaCoCo integration

* build.gradle.kts: move configuration values to `gradle.properties` file

* Bump `javax.xml.bind:jaxb-api` dependency to `2.4.0-b180830.0359`

* IntelliJInstrumentCodeTask.kt: make sure `instrumentedClassPath` exists before replacing it with `compiledClassPath`

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jakub Chrzanowski <jakub.chrzanowski@jetbrains.com>
@hsz hsz merged commit 9ce6d55 into JetBrains:master Sep 27, 2022
@hsz
Copy link
Member

hsz commented Sep 27, 2022

Thanks again, Matthew! That was a nice change to the project. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants