Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Provide a way to attach a mount of library as module deps #160

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

lizyxr
Copy link

@lizyxr lizyxr commented Nov 13, 2021

I need to create a fake gradle project with a huge amount of jar libs as deps. So I create this change. I think this would be useful since it's common to encounter freeze due to a project with a huge amount of libs. And we need to create such fake project for debugging.

user can 1 dummy jar lib to module0 by using "dummyLocalJarLibsDependency": [{"moduleName": "module0", "count": "1"}]. It will make sure the dummy jar file is generated locally i.e. you can sync project without issue.

xinruiy and others added 3 commits November 12, 2021 21:01
With this change, user can add fake jar files as module dpes. It will
create the dummy jar file in
module_root/libs/ directory and update the deps in build file. It allows
us to create a project with huge amount of jar libs deps which is useful
when we dubug freeze due to too many lib deps.

User can name the module that needs the dummy jar libaries and how many
jar libaries need to be created by using
"dummyLocalJarLibsDependency": [{"moduleName": "module0", "count":  "1"}]
@lizyxr
Copy link
Author

lizyxr commented Nov 13, 2021

I have update cla but not sure how can I ask for a rescan?

@borisf
Copy link
Contributor

borisf commented Nov 13, 2021

Thanks Liz,

Few thoughts on my end

  1. Please accept the contributor CLA
  2. Can we have a doc (could be an .md file) descriping this functionality, as other developers might find this feature usefull
  3. @NikitaKozlov please take a look

@borisf
Copy link
Contributor

borisf commented Nov 13, 2021

More thoughts:

  1. I will re-check the CLA
  2. Please fix the following tests

Task :aspoet:test

com.google.androidstudiopoet.generators.ModuleBuildBazelGeneratorTest > generator applies dependencies from the blueprint FAILED
org.mockito.exceptions.verification.junit.ArgumentsAreDifferent at ModuleBuildBazelGeneratorTest.kt:46

com.google.androidstudiopoet.generators.android_modules.AndroidModuleBuildBazelGeneratorTest > generator sets correct target name from the blueprint FAILED
org.mockito.exceptions.verification.junit.ArgumentsAreDifferent at AndroidModuleBuildBazelGeneratorTest.kt:109

com.google.androidstudiopoet.generators.android_modules.AndroidModuleBuildBazelGeneratorTest > generator applies libraries from the blueprint FAILED
org.mockito.exceptions.verification.junit.ArgumentsAreDifferent at AndroidModuleBuildBazelGeneratorTest.kt:65

com.google.androidstudiopoet.generators.android_modules.AndroidModuleBuildBazelGeneratorTest > generator sets correct android_binary rule class for Application module FAILED
org.mockito.exceptions.verification.junit.ArgumentsAreDifferent at AndroidModuleBuildBazelGeneratorTest.kt:37

com.google.androidstudiopoet.converters.ConfigPojoToAndroidModuleConfigConverterTest > convert passes correct values to result AndroidModuleConfig FAILED
java.lang.AssertionError at ConfigPojoToAndroidModuleConfigConverterTest.kt:109

com.google.androidstudiopoet.converters.ConfigPojoToAndroidModuleConfigConverterTest > convert result AndroidModuleConfig that has launch activity when index == 0 FAILED
java.lang.AssertionError at ConfigPojoToAndroidModuleConfigConverterTest.kt:118

@NikitaKozlov
Copy link
Contributor

@lizyxr Thank you for your PR, I think this change is useful.

I'm not sure if "Dummy" prefix is actually needed. From the stand point of the Android Studio Poet, this is a legitimate jar lib dependency, since it is not responsibility of Android Studio Poet to generate the jar. If my understanding is correct, then I would suggest to remove the "dummy" prefix from everywhere and just call it "localJarLibsDependency".

I agree with Boris, it would be useful to have this functionality documented.

@lizyxr
Copy link
Author

lizyxr commented Nov 19, 2021

Thanks Boris & Nikita for reviewing! I have fixed the test, remove the prefix and add some description in README.md. Do you think we need another .md for detailed function description?

@@ -56,6 +57,10 @@ will crawl the folder recursively and execute each config in turn.
* Specify `viewBinding` for View Binding.
* Specify `composeConfig` for Compose.
* It has a property `actionCount` to indicate the number of clickable actions.
* Sepcify `localJarLibsDependency` for local jar depencencies.
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify

* Sepcify `localJarLibsDependency` for local jar depencencies.
* It's a list of items that indicate creating local jar depencencies for specific module. For each item,
* It has a property `moduleName` to indicate the module name that depencies are attached to.
* It has a properoty `count` to indicate the number of depencies are attached.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a link to a sample config with Jars

Copy link
Contributor

Choose a reason for hiding this comment

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

dependencies

@borisf
Copy link
Contributor

borisf commented Nov 20, 2021

@lizyxr - thanks a lot
@NikitaKozlov - please review

Added some comments

===========================================

What would be our thoughts on:

  1. Havig a seprate md file for these feature + sample config
  2. Shall we generate the jars by ourselves, my concern is the user will provide the proper config but with out the jar files. Creating jars programmatically is pretty easy, https://sites.google.com/site/androcoding/classroom-news/howtocreatejarfileprogrammatically

* Sepcify `localJarLibsDependency` for local jar depencencies.
* It's a list of items that indicate creating local jar depencencies for specific module. For each item,
* It has a property `moduleName` to indicate the module name that depencies are attached to.
* It has a properoty `count` to indicate the number of depencies are attached.
Copy link
Contributor

Choose a reason for hiding this comment

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

A typo "property" instead of "properoty"

@NikitaKozlov
Copy link
Contributor

I also left a small comment about a typo. The rest - LGTM.
Regarding generation of jars by ourselves - I think is might be a good idea, but outside of the scope of this PR.

@borisf
Copy link
Contributor

borisf commented Nov 20, 2021

Ok, once the review comments are fixed, I will merge the change. In addition will open a bug(features request) for jar generation.

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

Successfully merging this pull request may close these issues.

3 participants