-
Notifications
You must be signed in to change notification settings - Fork 92
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
chore: keep hadoop-hdfs-client and hadoop-common version consistent #457
Conversation
hugegraph-loader/pom.xml
Outdated
@@ -307,7 +308,7 @@ | |||
<dependency> | |||
<groupId>org.apache.hadoop</groupId> | |||
<artifactId>hadoop-common</artifactId> | |||
<version>3.3.1</version> | |||
<version>${hdfs.version}</version> |
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.
hadoop.version
maybe better
Codecov Report
@@ Coverage Diff @@
## master #457 +/- ##
============================================
+ Coverage 62.51% 62.52% +0.01%
- Complexity 894 1867 +973
============================================
Files 91 260 +169
Lines 4396 9425 +5029
Branches 516 873 +357
============================================
+ Hits 2748 5893 +3145
- Misses 1445 3150 +1705
- Partials 203 382 +179 see 169 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -40,6 +40,7 @@ | |||
<release.name>${project.artifactId}</release.name> | |||
<final.name>apache-${release.name}-incubating-${project.version}</final.name> | |||
<jackson.version>2.12.3</jackson.version> | |||
<hadoop.version>3.3.1</hadoop.version> |
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.
Could check whether there are other similar Hadoop dependencies in the project at the same time. If there are multiple module used it, you can put the version attribute under the root to unify them (pom)
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.
I will do it later.
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.
I will do it later.
@chenlixuan We can do it in the next pr, unify the dependencies versions of the entire project.
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.
Need update the dependency file to fix the ci error @simon824 we should add a PR template and tell the devs how to modify it
hi @chenlixuan , in order to check the LICENSE conveniently, when we update the dependency, we need also update this file at the same time (https://github.com/chenlixuan/incubator-hugegraph-toolchain/blob/master/hugegraph-dist/scripts/dependency/known-dependencies.txt #L92) and make the version of dependencies in the file consistent with pom.xml |
refer to #456