Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
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
androidannotations:developandroidannotations/androidannotations:developfrom
WonderCsabo:1478_betterAndroidManifestFinderWonderCsabo/androidannotations:1478_betterAndroidManifestFinderCopy head branch name to clipboard
Sep 20, 2015
Merged

Use a better AndroidManifest.xml finder#1543
yDelouis merged 1 commit into
androidannotations:developandroidannotations/androidannotations:developfrom
WonderCsabo:1478_betterAndroidManifestFinderWonderCsabo/androidannotations:1478_betterAndroidManifestFinderCopy head branch name to clipboard

Conversation

@WonderCsabo

@WonderCsabo WonderCsabo commented Sep 11, 2015

Copy link
Copy Markdown
Member

Implements #1478.

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

@yDelouis yDelouis Sep 19, 2015

Copy link
Copy Markdown
Contributor

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

yDelouis commented Sep 19, 2015

Copy link
Copy Markdown
Contributor

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

WonderCsabo commented Sep 19, 2015

Copy link
Copy Markdown
Member Author

Done. Is this what you thought?

@yDelouis

yDelouis commented Sep 19, 2015

Copy link
Copy Markdown
Contributor

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

@WonderCsabo

WonderCsabo commented Sep 19, 2015

Copy link
Copy Markdown
Member Author

@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

yDelouis commented Sep 20, 2015

Copy link
Copy Markdown
Contributor

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.