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 shading support #162

Merged
merged 5 commits into from
Aug 26, 2015
Merged

Add shading support #162

merged 5 commits into from
Aug 26, 2015

Conversation

wxiang7
Copy link

@wxiang7 wxiang7 commented Aug 15, 2015

Add shading support, #156.
In this PR, a new assembly setting key is introduced, namely assemblyShadingRules
e.g.

assemblyShadingRules in assembly := Seq(
  Shader.rename("com.google.protobuf.**" -> "com.google.protobuf3.@1")
    .applyToCompiling
    .applyTo("com.google.protobuf", "protobuf-java", "3.0.0-alpha-3.1")
)

assemblyShadingRules consists of a sequence of shading rules. And there're three types of shading rules in total, they're: Shader.rename, Shader.remove, Shader.keepOnly. For each shading rule, it can be applied to a list of targets: applyToCompiling means applying it to current module, and applyTo means applying it to some dependency artifact.

These three types of shading rules originated from jarjar rules, for details please go to this link.

@eed3si9n
Copy link
Member

Awesome! Thanks for this contribution!

@@ -0,0 +1,18 @@
package org.pantsbuild.jarjar
Copy link
Member

Choose a reason for hiding this comment

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

Could we move this into package sbtassembly? If Pants is using that name, I don't want to collide with it.

Copy link
Author

Choose a reason for hiding this comment

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

I'm afraid we can't, since process method in JarProcessor is package private.
I'm not sure why they use such a method signature.

@eed3si9n
Copy link
Member

Do you mind adding some documentation on README?

(version.isEmpty || version.get == mod.revision)
}

case class ShadeRule(rule: String,
Copy link
Member

Choose a reason for hiding this comment

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

I feel like ShadeRule should a sealed trait, and Rename, Remove, KeepOnly should each be a case class under object ShadeRule. I can do that refactoring.

Copy link
Author

Choose a reason for hiding this comment

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

Will add more doc after all these refactoring :)

@wxiang7
Copy link
Author

wxiang7 commented Aug 20, 2015

hi @eed3si9n, could you please help review the refactored code when you're convenient. thanks :)

@eed3si9n
Copy link
Member

Refactoring looks good!
Do you mind adding a scripted test? Here's explanation of scripted - http://eed3si9n.com/testing-sbt-plugins

@wxiang7
Copy link
Author

wxiang7 commented Aug 26, 2015

hi @eed3si9n , scripted tests added.

@eed3si9n
Copy link
Member

Thanks again.

eed3si9n added a commit that referenced this pull request Aug 26, 2015
@eed3si9n eed3si9n merged commit 6e1db80 into sbt:master Aug 26, 2015
eed3si9n added a commit that referenced this pull request Sep 10, 2015
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.

2 participants