Handle autogenerated classes correctly. #527
Conversation
| @@ -149,8 +149,9 @@ class J2objcPlugin implements Plugin<Project> { | |||
| dependsOn: 'j2objcPreBuild') { | |||
| group 'build' | |||
| description "Translates all the java source files in to Objective-C using 'j2objc'" | |||
| // Default location for generated source files using annotation processor compilation. | |||
There was a problem hiding this comment.
Can you add a reference to where this is defined?
|
LGTM |
|
Waiting for @andrewfunston's ack on #517 . This is a guess, we'll see if it works. If it does indeed work, I'll try to look through Gradle's source to see if we can provide an authoritative reference. |
advayDev1
force-pushed
the
apt
branch
2 times, most recently
from
783c85c to
08ae497
Compare
advayDev1
changed the title
|
@brunobowden ptal; completely redone btw. |
|
Per @andrewfunston at #517 (comment) - this is functionally good to go |
| */ | ||
| List<String> generatedSourceDirs = new ArrayList<>() | ||
| // Default location for generated source files using annotation processor compilation. | ||
| List<String> generatedSourceDirs = ['build/classes/main'] |
There was a problem hiding this comment.
Is there any convention around writing generated source to build/classes/test?
There was a problem hiding this comment.
I haven't found anything like that. I don't think we could support distinct annotation processors in the test source set (that aren't in the main source set) at this moment without significant changes (remember we have a limitation that build-closure is not possible to apply for the test source).
There was a problem hiding this comment.
I personally used injection in my test set on my real project; I'm not sure if it's standard practice or not. I add component that inject test implementations (in memory dbs, etc); that was failure on my part to include that in the sample project, I was attempting to boil everything down pretty thin. Is there any actionable items for this, or is this an accepted shortcoming of the current j2objc plugin?
There was a problem hiding this comment.
How about we do the same as we did here?
i.e. you put out some GitHub/Gist repos of an issue (if there actually is any, it might just work?), and we'll try to fix. But let's please do it as a separate bug and PR (please file the bug Andy).
Adding build/classes/test here is not a good idea regardless, as it would add those test files to your production library.
It is the annotation processors themselves that we can't have be distinct for the test set. I think I can get it to work as long as you use for example Dagger in both main and test, but you happen to inject different classes (along with their autogen counterparts) in your test code. If you were to use SomeOtherInjectionFramework in test (only), I don't think we can make that work in the near future.
|
LGTM after addressing comments |
- Make autogenerated files work even without --build-closure - Handle changes to autogenerated files that are not solely caused by changes to hand-written files (ex. configuration changes). - Default to build/classes/main for annotation-processor generated source. - Both CycleFinder and Translate now treat autogen code identically. - Remove duplicate functionality between 'additional' and 'generated' files. Fixes j2objc-contrib#517
|
Unit-tests for all platforms have passed; non-aesthetic changes should have only affected them. Will get the 30min result on main branch anyway. merging. |
Handle autogenerated classes correctly.
Handle autogenerated classes correctly.
Fixes #517