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

Using TCK Tested JDK builds of OpenJDK #1604

Merged
merged 2 commits into from
Oct 28, 2021

Conversation

carldea
Copy link
Contributor

@carldea carldea commented Sep 10, 2021

The AdoptOpenJDK has been discontinued since July 2021. When using Zulu you get all the latest updated (TCK Tested) builds for all versions of OpenJDK. Also, added a fixed (major) version.

The AdoptOpenJDK has been discontinued since July 2021. When using Zulu you get all the latest updated (TCK Tested) builds for all versions of OpenJDK. Also, added a fixed (major) version.
@codecov
Copy link

codecov bot commented Sep 14, 2021

Codecov Report

Merging #1604 (c40231f) into master (f529c62) will increase coverage by 2.24%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1604      +/-   ##
============================================
+ Coverage     61.04%   63.28%   +2.24%     
- Complexity     6557     6704     +147     
============================================
  Files           418      418              
  Lines         34449    34449              
  Branches       4765     4764       -1     
============================================
+ Hits          21028    21800     +772     
+ Misses        11236    10402     -834     
- Partials       2185     2247      +62     
Impacted Files Coverage Δ
...c/main/java/com/baidu/hugegraph/util/JsonUtil.java 69.23% <0.00%> (-0.77%) ⬇️
...c/main/java/com/baidu/hugegraph/task/HugeTask.java 72.00% <0.00%> (-0.31%) ⬇️
...du/hugegraph/backend/tx/GraphIndexTransaction.java 84.18% <0.00%> (+0.21%) ⬆️
.../baidu/hugegraph/backend/query/ConditionQuery.java 86.38% <0.00%> (+0.27%) ⬆️
...va/com/baidu/hugegraph/task/ServerInfoManager.java 71.34% <0.00%> (+0.56%) ⬆️
...java/com/baidu/hugegraph/type/define/DataType.java 87.12% <0.00%> (+0.72%) ⬆️
.../backend/store/cassandra/CassandraSessionPool.java 57.14% <0.00%> (+1.02%) ⬆️
...egraph/backend/store/cassandra/CassandraShard.java 52.77% <0.00%> (+1.85%) ⬆️
...du/hugegraph/backend/store/BackendSessionPool.java 80.55% <0.00%> (+2.77%) ⬆️
.../baidu/hugegraph/backend/store/BackendSession.java 80.95% <0.00%> (+4.76%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f529c62...c40231f. Read the comment docs.

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@github-actions
Copy link

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

Also update the name of the and java version variable
@javeme javeme removed the inactive label Oct 26, 2021
@@ -21,12 +21,13 @@ jobs:
fail-fast: false
matrix:
BACKEND: [memory, cassandra, scylladb, hbase, rocksdb]
JAVA_VERSION: ['8']
Copy link
Contributor

Choose a reason for hiding this comment

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

please move the environment variable to line 20

Copy link
Member

Choose a reason for hiding this comment

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

matrix should define here (as official doc examine)

Copy link
Contributor

Choose a reason for hiding this comment

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

since we only need an environment variable instead of a matrix, so I think the variable is more suitable. of course both of these are ok, just keep matrix if we want to add tests for multiple java versions in the future.
image

@imbajin
Copy link
Member

imbajin commented Oct 27, 2021

thanks for contribution, and wonder to know why we choose zulu instead of temurin (as official suggestion below, what's the difference?)

image

@carldea
Copy link
Contributor Author

carldea commented Oct 27, 2021

thanks for contribution, and wonder to know why we choose zulu instead of temurin (as official suggestion below, what's the difference?)

The difference depends on your needs.
For example:
use-case 1:
Customers often will run apps on fixed build versions (archived version). At times the latest update of the JDK (build) can/has broken production apps. Azul’s Zulu has many archived versions.

Use case 2:
Migrating and using newer Java language features on a non-LTS release. Azul’s Zulu supports all non-LTS versions.

Note: The original setup-java@v1 GitHub action used Zulu by default because it supported every version of the OpenJDK. While Temurin is also a great choice for folks it mainly supports LTS versions since Sept2021.
After the new v2 folks currently really have 2 choices Temurin & Zulu.

I personally prefer consistency and a peace of mind. And if a vendor or company would like paid support there are choices.

image

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.

OK, use Zulu version now

And attach the official doc for others to refer

@javeme javeme merged commit 4b4b4b6 into apache:master Oct 28, 2021
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.

None yet

3 participants