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

Introduce keyTransformer property for PropertiesFileTransformer #247

Merged

Conversation

marcphilipp
Copy link
Contributor

The new keyTransformer property can be used to transform keys in properties files, e.g. because they contain class names about to be relocated. Its value can be set to a closure that receives the original key and returns the key name to be used in the resulting properties file.

We need this for junit-console-launcher because it uses joptsimple by @pholser which we want to relocate to a different package in order to enable users to test code that uses a different version of joptsimple. However, the joptsimple JAR contains properties files that contain keys that are class names.

@sbrannen
Copy link

Very elegant, and I'm really looking forward to seeing this in a release! 😉

@johnrengelman
Copy link
Collaborator

Thanks @marcphilipp for the PR. I'm still ingesting this, but off the top of my head, I'm thinking I might take this and extend it as a more 1st citizen of the processing engine so that you can register arbitrary actions to execute on each file being processed, or at least a new interface on transformer that allows you to register actions. That could help solve a few other issues that exist.

So, I'm not ignoring this, I'm just thinking about it and will be looking for time in my schedule to experiment with a few things.

@marcphilipp
Copy link
Contributor Author

@johnrengelman Any news on how to move this forward?

@johnrengelman johnrengelman added this to the 3.0.0 milestone Nov 18, 2016
@johnrengelman johnrengelman modified the milestones: 2.0.0, 3.0.0 Apr 5, 2017
The new keyTransformer property can be used to transform keys in
properties files, e.g. because they contain class names about to be
relocated. Its value can be set to a closure that receives the original
key and returns the key name to be used in the resulting properties
file.
@marcphilipp
Copy link
Contributor Author

I've rebased my branch onto master and updated this PR.

@johnrengelman johnrengelman merged commit 74a99b4 into GradleUp:master Apr 5, 2017
@marcphilipp
Copy link
Contributor Author

🎉🎉🎉

@johnrengelman
Copy link
Collaborator

johnrengelman commented Apr 5, 2017

@marcphilipp @sbrannen I merged this changed and pushed a snapshot build to https://oss.jfrog.org/artifactory/plugins-snapshot (2.0.0-SNAPSHOT).
You can give that shot if you'd like.

@sormuras
Copy link
Contributor

sormuras commented Apr 5, 2017

Thanks @johnrengelman , I will test it right away!

Btw. https://projects.ow2.org/bin/view/asm/asm-5-2-is-out with even more bug fixes. It would be great if you upgraded that dependency as well. I guess, it's only 2-times the 5.1 -> 5.2 in shadow/gradle/dependencies.gradle.

@sbrannen
Copy link

sbrannen commented Apr 5, 2017

Glad to see the merge... and the snapshot!

And an upgrade to ASM 5.2 would top it off. 😉

sormuras added a commit to junit-team/junit5 that referenced this pull request Apr 5, 2017
Remove superseded `PropertiesFileTransformer` from `buildSrc` folder;
its keyTransformer feature is integrated in Shadow now. For details see:

GradleUp/shadow#247
@sormuras
Copy link
Contributor

sormuras commented Apr 5, 2017

@johnrengelman See linked commit above. It works perfectly now in Java 8 and 9. Now "AssertJ" maven-shading cglib is our next show stopper: https://travis-ci.org/junit-team/junit5/builds/219055181

@marcphilipp marcphilipp deleted the properties-key-transformer branch July 2, 2017 08:25
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