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

refact: adapt apache package and dependency in all modules (Breaking Change) #2019

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

z7658329
Copy link
Member

@z7658329 z7658329 commented Nov 18, 2022

implement #1985

@z7658329
Copy link
Member Author

z7658329 commented Nov 18, 2022

image

also need to rebuild license? @simon824

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #2019 (22c4012) into master (44bea37) will increase coverage by 0.07%.
The diff coverage is 55.96%.

❗ Current head 22c4012 differs from pull request most recent head 64e7357. Consider uploading reports for the commit 64e7357 to get more accurate results

@@             Coverage Diff              @@
##             master    #2019      +/-   ##
============================================
+ Coverage     66.32%   66.39%   +0.07%     
  Complexity      976      976              
============================================
  Files           482      482              
  Lines         41463    41455       -8     
  Branches       5890     5888       -2     
============================================
+ Hits          27500    27526      +26     
+ Misses        11263    11232      -31     
+ Partials       2700     2697       -3     
Impacted Files Coverage Δ
...pi/src/main/java/org/apache/hugegraph/api/API.java 74.11% <ø> (ø)
.../java/org/apache/hugegraph/api/auth/AccessAPI.java 0.00% <ø> (ø)
.../java/org/apache/hugegraph/api/auth/BelongAPI.java 0.00% <ø> (ø)
...n/java/org/apache/hugegraph/api/auth/GroupAPI.java 0.00% <ø> (ø)
...n/java/org/apache/hugegraph/api/auth/LoginAPI.java 74.28% <ø> (ø)
...java/org/apache/hugegraph/api/auth/ProjectAPI.java 82.05% <ø> (ø)
.../java/org/apache/hugegraph/api/auth/TargetAPI.java 0.00% <ø> (ø)
...in/java/org/apache/hugegraph/api/auth/UserAPI.java 75.80% <ø> (ø)
...che/hugegraph/api/filter/AuthenticationFilter.java 55.00% <ø> (ø)
...ache/hugegraph/api/filter/CompressInterceptor.java 73.07% <ø> (ø)
... and 690 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

javeme
javeme previously approved these changes Nov 18, 2022
javeme
javeme previously approved these changes Nov 18, 2022
@imbajin imbajin mentioned this pull request Nov 18, 2022
15 tasks
@javeme
Copy link
Contributor

javeme commented Nov 19, 2022

@z7658329 can we move the CodeQL fixes into a separate PR?

@z7658329
Copy link
Member Author

z7658329 commented Nov 19, 2022

@z7658329 can we move the CodeQL fixes into a separate PR?

i have fixed, but in ci progress ,
hugegraph-ci / build (rocksdb, 11) (pull_request) failed, should rerun it , @javeme @imbajin see :
image

javeme
javeme previously approved these changes Nov 20, 2022
javeme
javeme previously approved these changes Nov 20, 2022
@imbajin
Copy link
Member

imbajin commented Nov 22, 2022

some TODOs & Warning:

  1. the APIVersion now is 0.67, shall we keep it?
  2. CommonVersion

@javeme why we need check common's version?
image

@imbajin imbajin changed the title adapt apache package and dependency refact: adapt apache package and dependency in all modules (Breaking Change) Nov 22, 2022
@javeme
Copy link
Contributor

javeme commented Nov 22, 2022

  1. the APIVersion now is 0.67, shall we keep it?
    • yes we need to keep it, api version must increase forever.
  2. why check common's version?
    • to avoid importing two different common jars with conflict versions.

@imbajin
Copy link
Member

imbajin commented Nov 22, 2022

why check common's version?
: to avoid importing two different common jars with conflict versions.

What does this mean? At present, the whole project seems to have only one huge-common dependency

@javeme
Copy link
Contributor

javeme commented Nov 22, 2022

What does this mean? At present, the whole project seems to have only one huge-common dependency

someone may import hg as a lib, at the same time also has imported hg-common.

@imbajin
Copy link
Member

imbajin commented Nov 22, 2022

What does this mean? At present, the whole project seems to have only one huge-common dependency

someone may import hg as a lib, at the same time also has imported hg-common.

Seems it should be handled by users(like use a fixed version or exclude the duplicate one), and currently it have 2 problems:

  1. the check fails when the version is 1.0.0 but the range is 2.x~2.x
  2. how to set the new version? (because the old common also use the 1.x & 2.x as their version num)

consider handle this in a separate PR

imbajin
imbajin previously approved these changes Nov 22, 2022
javeme
javeme previously approved these changes Nov 22, 2022
@@ -130,16 +82,16 @@

<dependencyManagement>
<dependencies>
<!-- hugegraph-commons -->
<!-- TODO: could we merge them to hugegraph-commons only? -->
Copy link
Contributor

Choose a reason for hiding this comment

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

no we can't merge them, because in some scenarios it is not necessary to import both the two packages at the same time, like client or loader.

Copy link
Member

Choose a reason for hiding this comment

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

OK, change the comment in next PR (could modify the common version together @z7658329 )

@z7658329
Copy link
Member Author

What does this mean? At present, the whole project seems to have only one huge-common dependency

someone may import hg as a lib, at the same time also has imported hg-common.

Seems it should be handled by users(like use a fixed version or exclude the duplicate one), and currently it have 2 problems:

  1. the check fails when the version is 1.0.0 but the range is 2.x~2.x
  2. how to set the new version? (because the old common also use the 1.x & 2.x as their version num)

consider handle this in a separate PR

@z7658329 z7658329 closed this Nov 22, 2022
@z7658329
Copy link
Member Author

z7658329 commented Nov 22, 2022

What does this mean? At present, the whole project seems to have only one huge-common dependency

someone may import hg as a lib, at the same time also has imported hg-common.

Seems it should be handled by users(like use a fixed version or exclude the duplicate one), and currently it have 2 problems:

  1. the check fails when the version is 1.0.0 but the range is 2.x~2.x
  2. how to set the new version? (because the old common also use the 1.x & 2.x as their version num)

consider handle this in a separate PR

image

image

so the VersionTest.testCoreVersionCheck successed! we need update commons jar? @javeme @imbajin

in dubbo project, use like this:
image

see : apache/incubator-hugegraph-commons#119

@z7658329 z7658329 reopened this Nov 22, 2022
@imbajin imbajin dismissed stale reviews from javeme and themself via f1db302 November 23, 2022 03:28
@imbajin imbajin force-pushed the feat/package_to_apache branch from c2dfb03 to f1db302 Compare November 23, 2022 03:28
@imbajin imbajin force-pushed the feat/package_to_apache branch from 22c4012 to 64e7357 Compare November 23, 2022 03:42
Copy link
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

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

use rebase and merge

@javeme javeme merged commit d340420 into apache:master Nov 23, 2022
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