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

Add new flattenMode called versionOnly #74

Closed
wants to merge 0 commits into from

Conversation

lasselindqvist
Copy link
Collaborator

versionOnly keeps everything in the pom except the version element. Version is expanded. This is very helpful for the CI friendly workflow (https://maven.apache.org/maven-ci-friendly.html) where version contains something like.${revision}

@hohwille
Copy link
Member

As it seems travis is having some SSL handshake issues whilst downloading with java7. Most probably because in java7 the wrong TLS version is used by default. We might needs some hacking of .travis.yml after maven central has been updated to reject old TLS. Nasty problems that seem not to be related to your code changes.

@hohwille
Copy link
Member

BTW: Would the name versionOnly not imply that only the version is kept and other things are stripped? Maybe onlyResolveVersion would be more clear for what it actually does...

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. Please consider my review suggestions.

descriptor.setRepositories( ElementHandling.keep );
descriptor.setScm( ElementHandling.keep );
descriptor.setUrl( ElementHandling.keep );
descriptor.setVersion( ElementHandling.expand );
Copy link
Member

Choose a reason for hiding this comment

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

adding a break here would be more clean than an uncommented fallthrough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do.

fatjar,

/** Only flattens the version field in pom. Keeps everything else. */
versionOnly;
Copy link
Member

Choose a reason for hiding this comment

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

as already commented: maybe onlyResolveVersion or onlyExpandVersion would be more self-explanatory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will change that.

@lasselindqvist
Copy link
Collaborator Author

I played around with this a bit and while this is suitable for single module projects, it is not very good for multi module ones. In those the children might have variables in their parent-version or dependencies-dependency-version that should probably be resolved.
So I'll change this solution a bit and might consider trying to find a solution that just resolves ${revision}, ${sha} and ${changelist} like @Antibrumm suggested. Something easy to implement would be a solution that in addition to resolving version, resolves parent and dependencies.

@lasselindqvist
Copy link
Collaborator Author

This https://github.com/mojohaus/flatten-maven-plugin/compare/master...lasselindqvist:Issue-63?expand=1 shows my thoughts on the issue. I haven't tested the solution yet at all but the idea is to extend abstract class AbstractStringBasedModelInterpolator which does not resolve variables that are not revision, sha or changelist. Requires testing and cleaning up.

@lasselindqvist
Copy link
Collaborator Author

I have a implementation for the CI friendly resolver and will publish once it and its integration test are all polished. It is based on replacing AbstractStringBasedModelInterpolator and StringSearchInterpolator to interpolate only the chosen variables.

@fmarot
Copy link

fmarot commented Sep 23, 2018

This PR has been closed but why ? It seems like the feature is not in an official branch yet... Am I wrong ?

@lasselindqvist
Copy link
Collaborator Author

I am not totally sure why this is closed (and it looks like by me). Perhaps because I am rearranging my forks branches and making a new pull request from a different branch soon.

@danhaywood
Copy link

danhaywood commented Sep 23, 2018 via email

@lasselindqvist
Copy link
Collaborator Author

Here you can find the new pull request #77

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.

4 participants