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

Build now uses shadow to relocate dependencies. #217

Closed
wants to merge 1 commit into from

Conversation

bigwaff
Copy link

@bigwaff bigwaff commented Jan 26, 2018

I incorporated the shadow plugin and relocated axion's dependencies. Build succeeds and unit/integration tests pass (although I am not sure whether a version of axion plugin with relocated dependencies is used for integration tests). I verified a version of axion with relocated dependencies (using grgit 1.7.2) allows my custom project which declares a dependency on grgit 2.1.0 to use axion w/out issues, i.e. the previous error which prompted this PR does not happen.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 61.967% when pulling 053dba8 on bigwaff:use-shadow into 124ab92 on allegro:master.

@adamdubiel
Copy link
Collaborator

I see the point, but i'm not sure i want to shadow all dependencies. I would rather make sure things work with newest GrGit.

@bigwaff
Copy link
Author

bigwaff commented Feb 1, 2018

I would rather make sure things work with newest GrGit.

There's a PR #215 for that ;-) However, newest GrGit requires Java 8. Maybe that is undesirable. If so, shadow allows axion-release to remain on Java 7 supported release of GrGit without forcing downstream clients to use latest GrGit and Java 8. Additionally, the classpath pollution not only effects downstream client users but other plugins and their dependencies as well. Relocating axion-release dependencies insulates axion-release, other plugins, and downstream client user build file logic.

Maybe a release of current axion-release plugin using shadow so user who wants to remain on Java 7 can do so w/out classpath pollution? And then follow on release of axion-release plugin with updated dependencies which require Java 8?

@bigwaff
Copy link
Author

bigwaff commented Feb 10, 2018

@adamdubiel Have you had a chance to consider my last comment? I have time this weekend to incorporate any feedback you have.

@adamdubiel
Copy link
Collaborator

adamdubiel commented Feb 11, 2018

@bigwaff i am done with a thing that i wanted to do a long time ago - remove GrGit dependency altogether from production code (but leave it in test code for convenience, at least for some time). This should fix any issues with shadowed dependencies, as the only ones left are JGit and JSch.

I also added missing tests, to avoid being surprised in runtime like you were when updating GrGit.

@adamdubiel
Copy link
Collaborator

PR is now open: #221

@adamdubiel
Copy link
Collaborator

As to Java 7 vs Java 8: Gradle 4.2 deprecated the support for Java 7.

@adamdubiel
Copy link
Collaborator

1.9.0 without GrGit is out now - i know it does not solve the problem completly (JSch and JGit are on shared classpath), but does it qualify to close this PR?

@adamdubiel
Copy link
Collaborator

adamdubiel commented Feb 28, 2018

1.9.0 without GrGit is out now - i know it does not solve the problem completly (JSch and JGit are on > shared classpath), but does it qualify to close this PR?

@bigwaff what do you think? ^

@bigwaff
Copy link
Author

bigwaff commented Mar 4, 2018

I think this PR is no longer necessary ;-)

@bigwaff bigwaff closed this Mar 4, 2018
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.

3 participants