Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Use a better AndroidManifest.xml finder #1543

Merged
yDelouis merged 1 commit into from
Sep 20, 2015
Merged

Use a better AndroidManifest.xml finder #1543

yDelouis merged 1 commit into from
Sep 20, 2015

Conversation

@WonderCsabo
Copy link
Member

@WonderCsabo WonderCsabo commented Sep 11, 2015

Implements #1478.

yDelouis
yDelouis reviewed Sep 19, 2015
View reviewed changes
...dannotations/src/main/java/org/androidannotations/internal/helper/AndroidManifestFinder.java Outdated
Copy link
Contributor

@yDelouis yDelouis Sep 19, 2015

Choose a reason for hiding this comment

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

I think you could add a method matches(String sourceGenerationFolder in the implementations of AndroidManifestFinderStrategy to avoid the if..else if..else. That way, you could create a list a AndroidManifestFinderStrategy, iterate it to find the right strategy. The constants XXX_GEN_FOLDER would be a inner constant of each strategy.

@yDelouis
Copy link
Contributor

yDelouis commented Sep 19, 2015

This is a great improvement. I have only one comment though. Could you address it ?

@WonderCsabo
Use a better AndroidManifest.xml finder
a411a69 
This new algorithm uses conventional paths of various build systems.
@WonderCsabo WonderCsabo force-pushed the 1478_betterAndroidManifestFinder branch from ef0490f to a411a69 Compare September 19, 2015 12:19
@WonderCsabo
Copy link
Member Author

WonderCsabo commented Sep 19, 2015

Done. Is this what you thought?

@yDelouis
Copy link
Contributor

yDelouis commented Sep 19, 2015

Yes, but I wouldn't have passed the variable sourcesGenerationFolder in the constructor but as a parameter of the method applies().

@WonderCsabo
Copy link
Member Author

WonderCsabo commented Sep 19, 2015

@yDelouis then we should also pass it in the findAndroidManifestFile(), and calculate the Matcher again with the found strategy. Or save it in the applies() method, but i think that would be bad design.
What would you like to do exactly? :)

@yDelouis
Copy link
Contributor

yDelouis commented Sep 20, 2015

You're right.

yDelouis added a commit that referenced this pull request Sep 20, 2015
@yDelouis
Merge pull request #1543 from WonderCsabo/1478_betterAndroidManifestF…
6133b7b 
…inder

Use a better AndroidManifest.xml finder
@yDelouis yDelouis merged commit 6133b7b into androidannotations:develop Sep 20, 2015
@WonderCsabo WonderCsabo deleted the 1478_betterAndroidManifestFinder branch September 20, 2015 11:49
@WonderCsabo WonderCsabo added this to the 4.0 milestone Sep 20, 2015
@dodgex dodgex mentioned this pull request Mar 9, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@WonderCsabo @yDelouis