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

MINOR: Remove unnecessary license generation code in wrapper.gradle #7742

Merged

Conversation

ijuma
Copy link
Member

@ijuma ijuma commented Nov 23, 2019

Newer versions of Gradle handle this automatically.

Credit to @granthenke for the tip.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Newer versions of Gradle handle this automatically.
@ijuma ijuma requested review from granthenke and omkreddy November 23, 2019 18:08
@omkreddy
Copy link
Contributor

omkreddy commented Nov 24, 2019

@ijuma For testing, I removed gradlew and ran gradle wrapper. The newly generated gradlew does not have license header. not sure if this is the expected behavior.

@ijuma
Copy link
Member Author

ijuma commented Nov 24, 2019

@omkreddy Are you sure? The diff for gradlew in this PR is after I did the same.

@ijuma
Copy link
Member Author

ijuma commented Nov 24, 2019

If you look at older branches, we were generating two license headers:

c19266a#diff-36ffbd2ea7085cf19547193a7faf30c8

Copy link
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

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

@ijuma Thanks for the PR. LGTM. earlier I was using older version of gradle (5.1.1) to generate wrapper. it was working with 5.6.2

@ijuma
Copy link
Member Author

ijuma commented Nov 24, 2019

Makes sense. Thanks for being diligent!

@ijuma ijuma merged commit 45842a3 into apache:trunk Nov 24, 2019
@ijuma ijuma deleted the remove-license-generation-from-gradle-script branch November 24, 2019 18:51
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