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

Remove the dependencies.lock from git and make them re-generated when building project #1180

Merged
merged 4 commits into from
Jul 13, 2020
Merged

Remove the dependencies.lock from git and make them re-generated when building project #1180

merged 4 commits into from
Jul 13, 2020

Conversation

openinx
Copy link
Member

@openinx openinx commented Jul 8, 2020

This patch will address the issue from #1176.

@openinx
Copy link
Member Author

openinx commented Jul 9, 2020

Ping @rdblue & @rdsr , PTAL. Thanks.

Copy link
Contributor

@rdsr rdsr left a comment

Choose a reason for hiding this comment

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

This looks correct to me as I think the dependency lock plugin will always generate the same dependencies.lock files if the versions.props file is not changed.

If would be great if @Fokko or @rdblue also had a look here

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

This looks good to me as well. I don't think the dependency.lock adds a lot of value. If there are any issues, then you would need to go deeper in the dependency tree anyway :)

build.gradle Outdated
@@ -103,6 +103,7 @@ subprojects {
compileJava {
options.encoding = "UTF-8"
}
compileJava.dependsOn(generateLock, saveLock)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the locking plugin as well, right?

@rdblue
Copy link
Contributor

rdblue commented Jul 9, 2020

I agree with doing this. Locking is really helpful when building a final app so you can ensure tests run with the same classpath that will be used in production. For a library like this, it makes far less sense. Huge lock files and confusing behavior when updating versions is not worth it.

Before merging, I think we should remove the dependency lock plugin as well.

@openinx
Copy link
Member Author

openinx commented Jul 10, 2020

+1

Copy link
Contributor

@aokolnychyi aokolnychyi left a comment

Choose a reason for hiding this comment

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

+1 from me too

@rdblue rdblue merged commit 063f926 into apache:master Jul 13, 2020
@rdblue
Copy link
Contributor

rdblue commented Jul 13, 2020

Looks like there isn't much more discussion. I'm going to merge this. Thanks @openinx and to everyone that took the time to review!

@rdblue rdblue linked an issue Jul 13, 2020 that may be closed by this pull request
@openinx openinx deleted the remove-generate-lock branch July 14, 2020 01:40
aokolnychyi pushed a commit to aokolnychyi/iceberg that referenced this pull request Aug 18, 2020
cmathiesen pushed a commit to ExpediaGroup/iceberg that referenced this pull request Aug 19, 2020
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.

scala version of dependencies.lock is incorrect in iceberg-flink
5 participants