Skip to content
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

Added the Manifest Update Mojo #52

Merged
merged 2 commits into from Sep 6, 2011

Conversation

nicstrong
Copy link
Contributor

Added a new mojo for updating various attributes in the manifest. There is some overlap with the VersionUpdateManifest here and will be happy to merge the 2 mojos if you think it is warranted.

Other methods were coincided for accomplishing the same results but were eventially discarded:

  1. aapt has various command line switches (e.g. --version-code and --version-name). This method was ruled out as it will only insert the attributes, not replace an existing attribute. This meant we could have some sensible defaults checked into source code control allowing the build to work outside environments without maven support.
  2. Using resource filtering. This has 2 problems. In eclipse it does not seem to be able to package the filtered manifest file, so complains that the maven property references are invalid in the POM. And using resource filtering it is only possible to do substitution, this is a problem on sharedUserId where it must be omitted if not required.

@mosabua
Copy link
Member

mosabua commented Aug 25, 2011

I think this should totally be merged with the versions update mojo. Otherwise there is too much overlap and it will be confusing for the user and potentially buggy if both are invoked..

@mosabua mosabua closed this Aug 25, 2011
@mosabua mosabua reopened this Aug 25, 2011
@nicstrong
Copy link
Contributor Author

Is there any project guidance/standards to merging mojos? For example should I keep the existing mojo name so existing POMs are not broken if we were to rename it? I guess the down side of this would be that the name version-update would be some what ambiguous as it would do a lot more after the merge. Maybe I should ask on the mailing list.

@mosabua
Copy link
Member

mosabua commented Aug 25, 2011

This will go out in the 3.0.0 alpha release. Do whatever makes the most sense from an API/configuration point of view. It should be clean and easy to understand. Asking on the mailing list is def a good idea.

@mosabua
Copy link
Member

mosabua commented Aug 30, 2011

At this stage we are on hold waiting for Nic's magic ;-)

…the existing version-tests to manifest-tests including tests for the new functionality
@nicstrong
Copy link
Contributor Author

Hi,

Finally got around to merging these 2 mojo's. Changes in the request are :

  1. I have rebased my manifestupdate-mojo branch on top of the latest master.
  2. I have merged the existing VersionUpdateMojo into the new ManifestUpdateMojo dropping the old versionNameUpdate flag as this is now covered by the more flexable versionName flag (which defaults to ${project.version} to emulate the older versionNameUpdate flag).
  3. Renamed all the existing tests from version-tests to manifest-tests and added new tests for all the extra code I added.

@mosabua
Copy link
Member

mosabua commented Sep 6, 2011

Awesome work!! Thanks.

mosabua added a commit that referenced this pull request Sep 6, 2011
Added the Manifest Update Mojo merging with existing Version update mojo.
@mosabua mosabua merged commit 8b9d27a into simpligility:master Sep 6, 2011
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.

None yet

3 participants