-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
HADOOP-18773. Upgrade maven-shade-plugin to 3.4.1 #5750
Conversation
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@ayushtkn thanks for the review, Please merge this PR too if possible as I don't have the write access for this repo. |
@pan3793 I am not sure how to verify if the issue is resolved with this upgrade. I think we can merge this PR since there are no blockers. |
whats the plan here? anyone has any comments? or should we merge this, I don't think this induces any new bugs or issues? Let me know your thoughts, if anyone wants me to hold merging this |
+1 for the merge. Regarding the problem is about source mismatch with the generated jars, AFAIK shaded binaries have that problem as the published source doesn't get rewritten. Any large shaded artifact (hello aws-sdk.jar) has this issue, at least in my experience. @pan3793 sorry about this, but I don't know how to fix it. I normally actually have the source trees of big projects, though that has its own problems. |
merged to trunk. @rohit-kb can you do a PR against branch 3.3 so we can merge it there too. thanks |
@steveloughran How do you think reverting HADOOP-16822? IMO, a wrong generated source jar is worse than none, at least the IDEA can jump correctly with de-compile when no source jar is associated. |
Thanks @steveloughran for the merge!. We can continue the remainder of the discussion initiated by pan3793 here. Meanwhile, I will try to create a PR for branch-3.3 too. Thanks again, Everyone! |
This is needed to successfully shade the hadoop binaries on recent Java versions. Contributed by Rohit Kumar
best to have that discussion on the jira, not a PR which hasn't caused the problem. |
This is needed to successfully shade the hadoop binaries on recent Java versions. Contributed by Rohit Kumar
This is needed to successfully shade the hadoop binaries on recent Java versions. Contributed by Rohit Kumar
Description of PR
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?