You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by "leerho (via GitHub)" <gi...@apache.org> on 2026/02/24 22:24:44 UTC
[PR] To main [datasketches-memory]
leerho opened a new pull request, #274:
URL: https://github.com/apache/datasketches-memory/pull/274
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2868030808
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,40 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
- os: [ windows-latest, ubuntu-latest, macos-latest ]
-
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
Review Comment:
This Copilot change is totally wrong!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2874057675
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,31 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
-
- steps:
- - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
- uses: actions/checkout@v4
- with:
- persist-credentials: false
-
- - name: Install Matrix JDK
- uses: actions/setup-java@v4
- with:
- java-version: ${{ matrix.jdk }}
- distribution: 'temurin'
- java-package: jdk
- architecture: x64
- cache: 'maven' # performance optimization
- overwrite-settings: true # ensures the runner's .m2/settings.xml is current
-
- - name: Verify Toolchain Configuration
- shell: bash
- run: |
- if [ -f ~/.m2/toolchains.xml ]; then
- cat ~/.m2/toolchains.xml
- else
- echo "toolchains.xml not found in ~/.m2/"
- fi
-
- - name: Echo Java & Maven Version
- run: mvn -version #also prints java-version
-
- - name: Print Current workflow
- run: cat .github/workflows/auto-jdk-os-matrix.yml
-
- - name: Test
- run: mvn clean test
-
- - name: Install
- run: mvn clean install "-DskipTests=true" "-Dmaven.clean.failOnError=false" #windows os hack
-
-
-# Architecture options: x86, x64, armv7, aarch64, ppc64le
-# Lifecycle: validate, compile, test, package, verify, install, deploy
-# -B batch mode, never stops for user input
-# -V show Version without stopping
-# -X debug mode
-# -q quiet, only show errors
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
+
+ # Reference the reusable workflow on the "workflows" branch
+ uses: apache/datasketches-memory/.github/workflows/reusable-jdk-os-matrix.yml@workflows
Review Comment:
According to GitHub Docs, the reusable workflows (RWFs) can have the location syntax:
{owner}/{repo}/.github/workflows/{filename}@{ref} #in a different repo
OR
./.github/workflows/{filename}@{ref} #in the same repo
where {ref} can be a branch, tag, or gitHash.
I would prefer having the RWFs in a separate branch ... or a common repo ... hmm...
I have it in a separate branch here, but these RWFs would be identical to what we would need for DS-Java.
Perhaps a better idea would be to put these in a "workflows" branch on DS-Java. It would be a natural common place for the various java repos. Since trying to share RWFs across different languages may be too cumbersome.
If I move this "workflows" branch to DS-java, would you approve?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2872166473
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,40 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
- os: [ windows-latest, ubuntu-latest, macos-latest ]
-
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
Review Comment:
Copilot often provides false positive comments :P
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,31 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
-
- steps:
- - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
- uses: actions/checkout@v4
- with:
- persist-credentials: false
-
- - name: Install Matrix JDK
- uses: actions/setup-java@v4
- with:
- java-version: ${{ matrix.jdk }}
- distribution: 'temurin'
- java-package: jdk
- architecture: x64
- cache: 'maven' # performance optimization
- overwrite-settings: true # ensures the runner's .m2/settings.xml is current
-
- - name: Verify Toolchain Configuration
- shell: bash
- run: |
- if [ -f ~/.m2/toolchains.xml ]; then
- cat ~/.m2/toolchains.xml
- else
- echo "toolchains.xml not found in ~/.m2/"
- fi
-
- - name: Echo Java & Maven Version
- run: mvn -version #also prints java-version
-
- - name: Print Current workflow
- run: cat .github/workflows/auto-jdk-os-matrix.yml
-
- - name: Test
- run: mvn clean test
-
- - name: Install
- run: mvn clean install "-DskipTests=true" "-Dmaven.clean.failOnError=false" #windows os hack
-
-
-# Architecture options: x86, x64, armv7, aarch64, ppc64le
-# Lifecycle: validate, compile, test, package, verify, install, deploy
-# -B batch mode, never stops for user input
-# -V show Version without stopping
-# -X debug mode
-# -q quiet, only show errors
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
+
+ # Reference the reusable workflow on the "workflows" branch
+ uses: apache/datasketches-memory/.github/workflows/reusable-jdk-os-matrix.yml@workflows
Review Comment:
Maybe we can simply inline the reuseable workflows at the main branch. Either under `.github/workflows` or under a new directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2849933726
##########
src/test/resources/testng.xml:
##########
@@ -0,0 +1,28 @@
+<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd">
+
Review Comment:
The TestNG suite references an external DTD URL in the DOCTYPE, which can introduce a network dependency (offline/air‑gapped builds may fail or stall) and can also be blocked by XML security hardening. Consider removing the DOCTYPE entirely or pointing to a bundled/local DTD so test execution doesn’t require outbound access.
```suggestion
```
##########
pom.xml:
##########
@@ -271,14 +279,18 @@ under the License.
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-failsafe-plugins.version}</version>
<configuration>
- <argLine>${jvm-arguments}</argLine>
+ <argLine>@{argLine} @{jvm.args}</argLine>
Review Comment:
Surefire `@{...}` late-property interpolation is primarily intended for `argLine` so JaCoCo can inject its agent; using `@{jvm.args}` (with dots in the property name) risks not being interpolated and being passed literally to the JVM. Safer approach is to keep `@{argLine}` but reference `${jvm.args}` normally so Maven resolves it reliably.
```suggestion
<argLine>@{argLine} ${jvm.args}</argLine>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2874057675
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,31 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
-
- steps:
- - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
- uses: actions/checkout@v4
- with:
- persist-credentials: false
-
- - name: Install Matrix JDK
- uses: actions/setup-java@v4
- with:
- java-version: ${{ matrix.jdk }}
- distribution: 'temurin'
- java-package: jdk
- architecture: x64
- cache: 'maven' # performance optimization
- overwrite-settings: true # ensures the runner's .m2/settings.xml is current
-
- - name: Verify Toolchain Configuration
- shell: bash
- run: |
- if [ -f ~/.m2/toolchains.xml ]; then
- cat ~/.m2/toolchains.xml
- else
- echo "toolchains.xml not found in ~/.m2/"
- fi
-
- - name: Echo Java & Maven Version
- run: mvn -version #also prints java-version
-
- - name: Print Current workflow
- run: cat .github/workflows/auto-jdk-os-matrix.yml
-
- - name: Test
- run: mvn clean test
-
- - name: Install
- run: mvn clean install "-DskipTests=true" "-Dmaven.clean.failOnError=false" #windows os hack
-
-
-# Architecture options: x86, x64, armv7, aarch64, ppc64le
-# Lifecycle: validate, compile, test, package, verify, install, deploy
-# -B batch mode, never stops for user input
-# -V show Version without stopping
-# -X debug mode
-# -q quiet, only show errors
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
+
+ # Reference the reusable workflow on the "workflows" branch
+ uses: apache/datasketches-memory/.github/workflows/reusable-jdk-os-matrix.yml@workflows
Review Comment:
According to GitHub Docs, the reusable workflows (RWFs) can have the location syntax:
{owner}/{repo}/.github/workflows/{filename}@{ref} #in a different repo
OR
./.github/workflows/{filename}@{ref} #in the same repo
where {ref} can be a branch, tag, or gitHash.
I would prefer having the reusable workflows in a separate branch ... or a common repo ... hmm...
I have it in a separate branch here, but these RWF would be identical to what we would need for DS-Java.
Perhaps a better idea would be to put these in a "workflows" branch on DS-Java. It would be a natural common place for the various java repos. Since trying to share RWFs across different languages may be too cumbersome.
If I move this "workflows" branch to DS-java, would you approve?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2874057675
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,31 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
-
- steps:
- - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
- uses: actions/checkout@v4
- with:
- persist-credentials: false
-
- - name: Install Matrix JDK
- uses: actions/setup-java@v4
- with:
- java-version: ${{ matrix.jdk }}
- distribution: 'temurin'
- java-package: jdk
- architecture: x64
- cache: 'maven' # performance optimization
- overwrite-settings: true # ensures the runner's .m2/settings.xml is current
-
- - name: Verify Toolchain Configuration
- shell: bash
- run: |
- if [ -f ~/.m2/toolchains.xml ]; then
- cat ~/.m2/toolchains.xml
- else
- echo "toolchains.xml not found in ~/.m2/"
- fi
-
- - name: Echo Java & Maven Version
- run: mvn -version #also prints java-version
-
- - name: Print Current workflow
- run: cat .github/workflows/auto-jdk-os-matrix.yml
-
- - name: Test
- run: mvn clean test
-
- - name: Install
- run: mvn clean install "-DskipTests=true" "-Dmaven.clean.failOnError=false" #windows os hack
-
-
-# Architecture options: x86, x64, armv7, aarch64, ppc64le
-# Lifecycle: validate, compile, test, package, verify, install, deploy
-# -B batch mode, never stops for user input
-# -V show Version without stopping
-# -X debug mode
-# -q quiet, only show errors
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
+
+ # Reference the reusable workflow on the "workflows" branch
+ uses: apache/datasketches-memory/.github/workflows/reusable-jdk-os-matrix.yml@workflows
Review Comment:
According to GitHub Docs, the reusable workflows can have the location syntax:
{owner}/{repo}/.github/workflows/{filename}@{ref} #in a different repo
OR
./.github/workflows/{filename}@{ref} #in the same repo
where {ref} can be a branch, tag, or gitHash.
I would prefer having the reusable workflows in a separate branch ... or a common repo ... hmm...
I have it in a separate branch here, but these RWF would be identical to what we would need for DS-Java.
Perhaps a better idea would be to put these in a "workflows" branch on DS-Java. It would be a natural common place for the various java repos. Since trying to share RWFs across different languages may be too cumbersome.
If I move this "workflows" branch to DS-java, would you approve?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2866910281
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,40 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
- os: [ windows-latest, ubuntu-latest, macos-latest ]
-
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
Review Comment:
The matrix is currently defined with only a `jdk` axis plus `include` entries that add `os`/`arch` but do not specify `jdk`. In GitHub Actions this produces an extra default job with no `matrix.os`/`matrix.arch`, and the included jobs may not have `matrix.jdk` set, causing the reusable workflow inputs (`jdk_version`, `os_type`, `java_arch`) to be empty/undefined. Define `os` as a real matrix axis (and map `arch` via `include`), or move `jdk` into each `include` entry (and drop the separate `jdk` axis) so every matrix row has all required keys.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2850149276
##########
pom.xml:
##########
@@ -271,14 +279,18 @@ under the License.
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-failsafe-plugins.version}</version>
<configuration>
- <argLine>${jvm-arguments}</argLine>
+ <argLine>@{argLine} @{jvm.args}</argLine>
Review Comment:
I'm not sure where Pilot is reading the source from, but the suggested change is already in the code!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2874057675
##########
.github/workflows/auto-jdk-os-matrix.yml:
##########
@@ -6,66 +6,31 @@ on:
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
pull_request:
paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/LICENSE', '**/NOTICE' ]
- # The branches below must be a subset of the branches above
branches: [ 'main', '[0-9]+.[0-9]+.[Xx]', 'v[0-9]+.[0-9]+.[Xx]']
- workflow_dispatch:
+ workflow_dispatch:
-env:
- MAVEN_OPTS: -Xmx4g -Xms1g
- MAVEN_ARGS: "-B -Dmaven.javadoc.skip=true -Dgpg.skip=true"
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
jobs:
- build:
- name: Build, Test, Install
- runs-on: ${{ matrix.os }}
-
+ run-matrix:
strategy:
fail-fast: false
matrix:
jdk: [ 25 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
-
- steps:
- - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
- uses: actions/checkout@v4
- with:
- persist-credentials: false
-
- - name: Install Matrix JDK
- uses: actions/setup-java@v4
- with:
- java-version: ${{ matrix.jdk }}
- distribution: 'temurin'
- java-package: jdk
- architecture: x64
- cache: 'maven' # performance optimization
- overwrite-settings: true # ensures the runner's .m2/settings.xml is current
-
- - name: Verify Toolchain Configuration
- shell: bash
- run: |
- if [ -f ~/.m2/toolchains.xml ]; then
- cat ~/.m2/toolchains.xml
- else
- echo "toolchains.xml not found in ~/.m2/"
- fi
-
- - name: Echo Java & Maven Version
- run: mvn -version #also prints java-version
-
- - name: Print Current workflow
- run: cat .github/workflows/auto-jdk-os-matrix.yml
-
- - name: Test
- run: mvn clean test
-
- - name: Install
- run: mvn clean install "-DskipTests=true" "-Dmaven.clean.failOnError=false" #windows os hack
-
-
-# Architecture options: x86, x64, armv7, aarch64, ppc64le
-# Lifecycle: validate, compile, test, package, verify, install, deploy
-# -B batch mode, never stops for user input
-# -V show Version without stopping
-# -X debug mode
-# -q quiet, only show errors
+ include:
+ - os: windows-latest
+ arch: x64
+ - os: ubuntu-latest
+ arch: x64
+ - os: macos-latest
+ arch: aarch64
+
+ # Reference the reusable workflow on the "workflows" branch
+ uses: apache/datasketches-memory/.github/workflows/reusable-jdk-os-matrix.yml@workflows
Review Comment:
According to GitHub Docs, the reusable workflows (RWFs) can have the location syntax:
{owner}/{repo}/.github/workflows/{filename}@{ref} #in a different repo
OR
./.github/workflows/{filename}@{ref} #in the same repo
where {ref} can be a branch, tag, or gitHash.
I would prefer having the RWFs in a separate branch ... or a common repo ... hmm...
I have it in a separate branch here, but these RWFs would be identical to what we would need for DS-Java.
Perhaps a better idea would be to put these in a "workflows" branch on DS-Java. It would be a natural common place for all the various java repos. Since trying to share RWFs across different languages may be too cumbersome.
Actually, I'd prefer if you would approve it here so I can test it. And then if it works, I'll move the "workflows" directory to DS-Java, where it makes more sense and can be leveraged by other DS-"java" repos, like hive, pig, characterization, vector, server, etc (that is, if we want to upgrade them :) )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2850133819
##########
src/test/resources/testng.xml:
##########
@@ -0,0 +1,28 @@
+<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd">
+
Review Comment:
Will fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] To main [datasketches-memory]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho merged PR #274:
URL: https://github.com/apache/datasketches-memory/pull/274
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org