Skip to content

Only provide actionable module structure advice #1417

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joshfriend
Copy link
Contributor

The threshold of 2 uses of android features is totally arbitrary and not helpful in practice.

This is especially true for the "Has Android library dependencies" advice, for which there is really no workaround and happens extremely frequently in our build.

The threshold of 2 uses of android features is totally arbitrary and not helpful in practice.
This is especially true for the "Has Android library dependencies" advice, for which there is really no workaround and happens extremely frequently in our build.
@joshfriend joshfriend force-pushed the jfriend/dont-fail-module-structure-for-android-deps branch 2 times, most recently from 8b908fe to bed0e43 Compare April 16, 2025 21:04
Copy link
Owner

@autonomousapps autonomousapps left a comment

Choose a reason for hiding this comment

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

I agree that the threshhold of 2 is arbitrary. Is it your position that the concept of a score is, per se, meaningless, and it's simply a binary yes/no situation?

@joshfriend
Copy link
Contributor Author

it's simply a binary yes/no situation?

Yep, that is my view. I did not remove all of the scoring code, but if we agree on this I probably should.

@autonomousapps
Copy link
Owner

autonomousapps commented Apr 28, 2025

The rationale for a score was that there are certain Android features that users tend to cargo cult that they aren't using, and so I wanted to flag them as something that could be deleted et voilà --> JVM project. For example, lots of Android modules (in the past, anyway), have a BuildConfig.java that they aren't using, or there might be template-generated resources (colors and strings) that they aren't using. Maybe I need to be more explicit about that, vs trying to calculate a "score"? Are my assumptions here still valid (in 2025)?

@joshfriend
Copy link
Contributor Author

Android features that users tend to cargo cult that they aren't using

But currently the plugin does not detect if some features are actually being used (like buildconfig), it only detects that they are enabled, which IMO is not useful (especially in square's build where we disable everything by default and have other checks to determine if you are using resources or not for example).

I think I'm trying to make the argument that the status quo is not useful to any builds, and my proposal here is at least useful to square 😄.

@timoloewe
Copy link
Contributor

timoloewe commented Apr 29, 2025

Hi! I want to share the same feedback from our experience. We are currently introducing DAGP in our modularized project and are also getting a lot of :

Module structure advice
This project uses limited Android features and could be a JVM project.
* Has Android library dependencies.

Those are all not actionable in any way, only noisy and, initially, confusing.

or there might be template-generated resources (colors and strings) that they aren't using

It sounds to me that it should be the task of other tools to flag such issues (like Android Lint).

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

Successfully merging this pull request may close these issues.

3 participants