Skip to content

[BEAM-4393]: Copy root repo's LICENSE & NOTICE into shadowJar#5472

Merged
kennknowles merged 1 commit intoapache:masterfrom
boyuanzz:build_copyright
May 24, 2018
Merged

[BEAM-4393]: Copy root repo's LICENSE & NOTICE into shadowJar#5472
kennknowles merged 1 commit intoapache:masterfrom
boyuanzz:build_copyright

Conversation

@boyuanzz
Copy link
Contributor

@apilloud
Copy link
Member

LGTM! I verified that this fixes BEAM-4393.

Only one question: should this go in jar instead of shadowJar?

@boyuanzz
Copy link
Contributor Author

Hey @apilloud , my understanding so far is, we use shadowJar to rewrite Jar. How do you feel like? @lukecwik

@apilloud
Copy link
Member

I believe we run the jar rule to build a jar with only our code, then we run the shadowJar rule to bundle that jar with third party code. This only needs to be in shadowJar, but that leaves our code in jar without a LICENSE file.

@kennknowles
Copy link
Member

kennknowles commented May 24, 2018

That seems like a problem. The shadowJar is the real thing and the "normal" jar is just an intermediate calculation.

@boyuanzz
Copy link
Contributor Author

I didn't find any Jar{} closure so far in https://github.com/apache/beam/blob/master/build_rules.gradle, which means, we possibly use the default configuration. I'm not sure whether we need LICENSE in Jar.

@kennknowles
Copy link
Member

Yea, OK. I didn't mean this PR is wrong. It seems this PR does put the LICENSE in the actual thing we want to ship to users. I guess the rest has to do with the release process.

@kennknowles kennknowles merged commit 1e2218a into apache:master May 24, 2018
@kennknowles
Copy link
Member

Merging in the interest of moving things slightly better. If our current release process is mostly right, then this PR fixes it.

@boyuanzz boyuanzz deleted the build_copyright branch July 23, 2018 20:06
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