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.

Sign up for GitHub

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

Jump to bottom

[Dependency Analysis] Remove Unused Dependencies #1090

Merged
merged 11 commits into from
Aug 19, 2024
Merged

[Dependency Analysis] Remove Unused Dependencies #1090

merged 11 commits into from
Aug 19, 2024

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Aug 14, 2024 •
edited
Loading

Project Thread: paaHJt-6YV-p2

Description

Based on the unused related advises generated by the Dependency Analysis Gradle plugin, this PR removes all unused dependencies from this project.

FYI: As part of this change, the Dependency Analysis Gradle plugin got also updated to its latest 1.33.0 version.

Testing Steps

  1. Tooling:
    • Run the ./gradlew buildHealth task and verify that there is no unused related advise remaining within both reports, JSON and txt included:
      • build-health-report.json -> Search for unusedCount
      • build-health-report.txt -> Search for unused
    • Check the manually triggered scheduled build and verify that everything works as expected with this newer version of the Dependency Analysis Gradle plugin (1.33.0).
  2. Testing:
    • Smoke test the sample app, try out all available screens and functionality. Verify everything is working as expected.
    • Also, if you want to be thorough about reviewing the changes, you could quickly smoke test, either the JP/WPAndroid and/or WCAndroid, with this version of Aztec, or via composite builds, and see if it works as expected.

@ParaskP7
Deps: Update dependency analysis to 1.33.0
796dec4 
Release Notes: https://github.com/autonomousapps/
dependency-analysis-gradle-plugin/blob/main/CHANGELOG.md#version-1330
@ParaskP7
Deps: Remove kotlinx coroutines from app
0bf2a1c 
These unused dependencies got added to this module via this
(56a0534) commit, which amongst
other changes, moved the media placeholders into it's own module.
However, these dependencies never got to be used and as such can be
safely removed.

------------------------------------------------------------------------

This removal was suggested by the dependency analysis report, see below:

Advice for :app
Unused dependencies which should be removed:
  implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4'
  ...

Dependencies which should be removed or changed to runtime-only:
  runtimeOnly 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4'
   (was implementation)
@ParaskP7
Build: Update dependency analysis configuration on app
0136921 
These 'unused' related advises for the 'app' module is actually
correct, both 'media-placeholders' and ':picasso-loader', can be removed
as dependencies since they are actually unused and not being utilized
from within the app.

However, I chose to keep them there and suppress the advise instead in
order to preserve the app default configuration, its purpose being to
test all those individual modules:
- aztec
- glide-loader
- picasso-loader
- wordpress-comments
- wordpress-shortcodes
- media-placeholders

As such this advice is instead suppressed and the 'onUnusedDependencies'
exclusion configuration were used to achieve that.

------------------------------------------------------------------------

Advice for :app
Unused dependencies which should be removed:
  implementation project(':media-placeholders')
  implementation project(':picasso-loader')
@ParaskP7
Deps: Remove all unused android test dependencies from aztec
a394608 
It is unclear why any 'androidTest' related dependency got
added to this module, as one can only navigate to this
(ad68cc9) commit and see that this was
done as part of the AndroidX migration, but, without any 'androidTest'
source set folder included on this module, nor any accompanying UI tests
added to it.

------------------------------------------------------------------------

This removal was suggested by the dependency analysis report, see below:

Advice for :aztec
Unused dependencies which should be removed:
  androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
  androidTestImplementation 'androidx.test.ext:junit:1.1.1'
  androidTestImplementation 'androidx.test:runner:1.2.0'
  ...

Dependencies which should be removed or changed to runtime-only:
  androidTestRuntimeOnly 'androidx.test:core:1.4.0'
   (was androidTestImplementation)
  ...
@ParaskP7
Deps: Replace androidx legacy support v4 dependency in aztec
67c07fd 
Although removing the 'androidx.legacy.support.v4' is enough by itself
and the build is (almost) not failing, it is better to also replace that
with 'androidx.collection' due to this 'androidx.collection.ArrayMap'
import on 'AztecQuoteSpan'.

FYI: It got introduced as part of this
(ad68cc9) commit, a part of the
AndroidX migration.

PS: To make sure that the build is not failing the only instance of
'androidx.legacy', the '<androidx.legacy.widget.Space/>' got replaced
with '<Space/>' on the 'aztec_format_bar_advanced.xml' view.

------------------------------------------------------------------------

This replacement was suggested by the dependency analysis report,
see below:

Advice for :aztec
Unused dependencies which should be removed:
  implementation 'androidx.legacy:legacy-support-v4:1.0.0'

These transitive dependencies should be declared directly:
  ...
  implementation 'androidx.collection:collection:1.1.0'
  ...
  implementation 'androidx.legacy:legacy-support-core-ui:1.0.0'
  ...
@ParaskP7
Build: Update dependency analysis configuration on media placeholders
7db2a43 
This 'unused' related advise for the 'media-placeholders' module is
actually incorrect as the build is failing when the 'appcompat'
dependency is removed.

This is happening due to the fact that 'aztec' has classes like
'AztecText', which is being used on this module, but the Dependency
Analysis plugin cannot understand that although there are no 'appcompat'
related imports, those are actually needed nevertheless. Another
dependency configuration might be able to solve this, but this was
considered as being outside the scope of this change.

------------------------------------------------------------------------

Advice for :media-placeholders
Unused dependencies which should be removed:
  implementation 'androidx.appcompat:appcompat:1.1.0'
@ParaskP7
Deps: Replace androidx legacy support v4 dependency in picasso loader
9794a6e 
Although removing the 'androidx.legacy.support.v4' is enough by itself
and the build is not failing, it is better to also replace that with
'androidx.collection' due to this 'androidx.collection.ArrayMap' import
on 'PicassoImageLoader'.

FYI: It got introduced as part of this
(ad68cc9) commit, a part of the
AndroidX migration.

------------------------------------------------------------------------

This replacement was suggested by the dependency analysis report,
see below:

Advice for :picasso-loader
Unused dependencies which should be removed:
  implementation 'androidx.legacy:legacy-support-v4:1.0.0'
  ...

These transitive dependencies should be declared directly:
  implementation 'androidx.collection:collection:1.1.0'
@ParaskP7
Deps: Replace android material with androidx appcompat in picasso loader
9289a45 
Removing the 'android.material' dependency wasn't enough by itself as
the build was then failing. Adding the transient 'androidx.appcompat'
dependency was actually required in order to make the build successful
again.

Then, and just like it was done on this
(7db2a43) change, the seemingly unused
'androidx.appcompat' dependency advise got suppressed as well.

------------------------------------------------------------------------

This replacement was suggested by the dependency analysis report,
see below:

Advice for :picasso-loader
Unused dependencies which should be removed:
  implementation 'com.google.android.material:material:1.2.1'
@ParaskP7
Deps: Remove androidx legacy dependency from wordpress comments
4c7a164 
This dependency is completely unused.

FYI: It got introduced as part of this
(ad68cc9) commit, a part of the
AndroidX migration.

------------------------------------------------------------------------

This removal was suggested by the dependency analysis report, see below:

Advice for :wordpress-comments
Unused dependencies which should be removed:
  implementation 'androidx.legacy:legacy-support-v4:1.0.0'
@ParaskP7
Deps: Replace android material with androidx appcompat in wordpress cmt
e578ad1 
Removing the 'android.material' dependency wasn't enough by itself as
the build was then failing. Adding the transient 'androidx.appcompat'
dependency was actually required in order to make the build successful
again.

Then, and just like it was done on this
(7db2a43) change, the seemingly unused
'androidx.appcompat' dependency advise got suppressed as well.

------------------------------------------------------------------------

This replacement was suggested by the dependency analysis report,
see below:

Advice for :wordpress-comments
Unused dependencies which should be removed:
  implementation 'com.google.android.material:material:1.2.1'
@ParaskP7
Build: Update dependency analysis configuration on wordpress shortcodes
48e2716 
This 'unused' related advise for the 'wordpress-shortcodes' module is
actually incorrect as the build is failing when the 'appcompat'
dependency is removed.

Just like it was done on this (7db2a43)
change, the seemingly unused 'androidx.appcompat' dependency advise got
suppressed as well.

------------------------------------------------------------------------

Advice for :wordpress-shortcodes
Unused dependencies which should be removed:
  implementation 'androidx.appcompat:appcompat:1.1.0'
@ParaskP7 ParaskP7 marked this pull request as ready for review August 14, 2024 12:49
@ParaskP7 ParaskP7 requested a review from wzieba August 14, 2024 12:49
@ParaskP7 ParaskP7 merged commit 0c56aec into trunk Aug 19, 2024
15 checks passed
@ParaskP7 ParaskP7 deleted the build/remove-unused-dependencies branch August 19, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wzieba wzieba wzieba approved these changes

Assignees
No one assigned
Labels
Core Tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ParaskP7 @wzieba