Skip to content

Add shading support#162

Merged
eed3si9n merged 5 commits intosbt:masterfrom
wxiang7:shader
Aug 26, 2015
Merged

Add shading support#162
eed3si9n merged 5 commits intosbt:masterfrom
wxiang7:shader

Conversation

@wxiang7
Copy link
Copy Markdown

@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
Copy Markdown
Member

Awesome! Thanks for this contribution!

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Do you mind adding some documentation on README?

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

wxiang7 commented Aug 20, 2015

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

@eed3si9n
Copy link
Copy Markdown
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
Copy Markdown
Author

wxiang7 commented Aug 26, 2015

hi @eed3si9n , scripted tests added.

@eed3si9n
Copy link
Copy Markdown
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