Update github actions by rflamand · Pull Request #8 · ImagingDataCommons/libdicomicc

rflamand · 2025-12-11T10:25:55Z

Update the github actions in this repository. The updates include:

Simplifying the existing run_unit_test.yml using pre-existing actions. These actions will setup emsdk.
Fix version for emsdk to 4.0.19. Also update BUILDING to represent this.
Create new action publish.yml that will publish a package to npm when a new release has been published. This will require the NODE_AUTH_TOKEN to be set in the project's secrets. Note that I've added some guard rails, mainly this actions will only run if the project is not a fork and the owner is ImagingDataCommons.

This PR tackles #7 .

rflamand · 2026-01-27T15:51:32Z

@fedorov Is there anyone who can review this PR (and the other)?

Copilot

Pull request overview

This PR updates GitHub Actions workflows to modernize CI/CD infrastructure and add automated npm publishing. The changes address issue #7 by implementing a publish workflow that will automatically publish the package to npm when a release is created. The Emscripten SDK version is also updated from 1.39.4 to 4.19.0.

Changes:

Simplified run_unit_tests.yml to use pre-existing actions for emsdk, Node.js, and dependency caching
Updated Emscripten SDK version from 1.39.4 to 4.19.0 in documentation
Added new publish.yml workflow to automatically publish npm packages on release with appropriate guard rails

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
BUILDING.md	Updated documented Emscripten SDK version from 1.39.4 to 4.19.0
.github/workflows/run_unit_tests.yml	Modernized workflow using pre-existing actions, simplified emsdk setup, updated action versions
.github/workflows/publish.yml	New workflow for automated npm package publishing on release with repository ownership guards

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.github/workflows/run_unit_tests.yml

.github/workflows/publish.yml

.github/workflows/run_unit_tests.yml

.github/workflows/publish.yml

.github/workflows/run_unit_tests.yml

fedorov · 2026-01-27T16:34:03Z

@rflamand can you please review comments from Copilot?

rflamand · 2026-01-28T10:07:15Z

@rflamand can you please review comments from Copilot?

@fedorov thanks for the swift response. I've updated the code based on copilot's review.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T11:54:56Z

.github/workflows/publish.yml

+          with:
+            version: 4.0.19
+            actions-cache-folder: '.emsdk-cache'
+


This line appears to contain trailing whitespace. Remove the trailing spaces for cleaner formatting and consistency with YAML best practices.

Copilot · 2026-01-28T11:54:56Z

.github/workflows/run_unit_tests.yml

+    - name: Install dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y liblcms2-dev autoconf


CMake is required for the build (minimum version 3.16 per BUILDING.md) but is not explicitly installed in the dependencies step. While ubuntu-latest typically includes cmake, it's better to explicitly install it to ensure the required version is available and to make the dependency explicit. Add cmake to the apt-get install command on line 44.

Suggested change

sudo apt-get install -y liblcms2-dev autoconf

sudo apt-get install -y liblcms2-dev autoconf cmake

Copilot · 2026-01-28T11:54:57Z

.github/workflows/publish.yml

+        - name: Install dependencies
+          run: |
+            sudo apt-get update
+            sudo apt-get install -y liblcms2-dev autoconf


CMake is required for the build (minimum version 3.16 per BUILDING.md) but is not explicitly installed in the dependencies step. While ubuntu-latest typically includes cmake, it's better to explicitly install it to ensure the required version is available and to make the dependency explicit. Add cmake to the apt-get install command on line 48.

Suggested change

sudo apt-get install -y liblcms2-dev autoconf

sudo apt-get install -y liblcms2-dev autoconf cmake

Copilot · 2026-01-28T11:54:57Z

.github/workflows/run_unit_tests.yml

+      run: |
+        sudo apt-get update
+        sudo apt-get install -y liblcms2-dev autoconf
+


This line appears to contain trailing whitespace. Remove the trailing spaces for cleaner formatting and consistency with YAML best practices.

Suggested change

Copilot · 2026-01-28T11:54:57Z

.github/workflows/publish.yml

+          run: |
+            sudo apt-get update
+            sudo apt-get install -y liblcms2-dev autoconf
+


This line appears to contain trailing whitespace. Remove the trailing spaces for cleaner formatting and consistency with YAML best practices.

Suggested change

Copilot · 2026-01-28T11:54:57Z

.github/workflows/run_unit_tests.yml

+        cache: 'yarn'
+        cache-dependency-path: |
+          package.json
+          yarn.lock


The yarn cache configuration references yarn.lock in the cache-dependency-path, but this file does not exist in the repository. The cache will not work correctly without a lock file. Either remove the cache configuration or ensure yarn.lock is committed to the repository. If you want to use yarn caching, run yarn install locally to generate yarn.lock and commit it.

There are no dependencies so caching doesn't actually make sense. Removed the caching fields.

Copilot · 2026-01-28T11:54:57Z

.github/workflows/run_unit_tests.yml

+        cache: 'yarn'
+        cache-dependency-path: |
+          package.json
+          yarn.lock


The yarn cache configuration references yarn.lock in the cache-dependency-path, but this file does not exist in the repository. The cache will not work correctly without a lock file. Either remove the cache configuration or ensure yarn.lock is committed to the repository. If you want to use yarn caching, run yarn install locally to generate yarn.lock and commit it.

There are no dependencies so caching doesn't actually make sense. Removed the caching fields.

Copilot · 2026-01-28T11:54:58Z

.github/workflows/publish.yml

+        - name: Publish to npm
+          run: |
+            npm publish .


The publish workflow does not verify that the version in package.json matches the release tag before publishing. Consider adding a validation step after checkout to ensure version consistency. For example, extract the version from package.json and compare it with the GitHub release tag to prevent publishing incorrect versions.

Copilot · 2026-01-28T11:54:58Z

.github/workflows/run_unit_tests.yml

-          llvm \
-          node \
-          yarn
+          llvm


CMake is required for the build (minimum version 3.16 per BUILDING.md) but is not explicitly installed in the dependencies step. While macos-latest typically includes cmake, it's better to explicitly install it to ensure the required version is available and to make the dependency explicit. Add cmake to the brew install command.

Suggested change

llvm

llvm \

cmake

Copilot · 2026-01-28T11:54:58Z

.github/workflows/run_unit_tests.yml

+      with:
+        version: 4.0.19
+        actions-cache-folder: '.emsdk-cache'
+


This line appears to contain trailing whitespace. Remove the trailing spaces for cleaner formatting and consistency with YAML best practices.

Suggested change

fedorov · 2026-01-28T12:08:15Z

@rflamand more issues identified ... don't ask me why it didn't complain about cmake and lock earlier ...

Also: I have never ever published an npm package from scratch. There is no npm package for this one yet. Do you know if this is supposed to work and create that package "de novo"?

For the dicom-microscopy-viewer package, I create a token that has permission limited to that package. But in this case, there is no existing package to limit permissions, so it would need to have unlimited permissions.

@igoroctaviano maybe you know?

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rflamand · 2026-01-28T14:25:50Z

@rflamand more issues identified ... don't ask me why it didn't complain about cmake and lock earlier ...

Also: I have never ever published an npm package from scratch. There is no npm package for this one yet. Do you know if this is supposed to work and create that package "de novo"?

For the dicom-microscopy-viewer package, I create a token that has permission limited to that package. But in this case, there is no existing package to limit permissions, so it would need to have unlimited permissions.

@igoroctaviano maybe you know?

I'm not 100% sure on this, I have also never published an npm repo from scratch. However, this is already published on npm, no? I would think that this is the npm package or do you mean something else?

pieper · 2026-01-28T14:47:45Z

I recently updated the npm publishing part of dcmjs if you want to look at a recent example github action.

rflamand · 2026-01-28T14:57:39Z

I recently updated the npm publishing part of dcmjs if you want to look at a recent example github action.

Interesting, had not heard of semantic release before. @fedorov is this something we also want for libdicomicc or will we keep it at manual releases (for now)?

fedorov · 2026-01-28T15:27:15Z

However, this is already published on npm, no? I would think that this is the npm package or do you mean something else?

OH! I missed that! So this and other related packages were originally developed by @hackermd when he was at MGH. He since moved to industry, and added me to the packages that we took under the IDC umbrella. Apparently, this specific package was missed... I am afraid we won't be able to do anything unless I get a response from Markus. I will email him right away to either transfer this one to me, or add me as a maintainer... Sorry I didn't realize this until now!

fedorov · 2026-01-28T15:34:43Z

I recently updated the npm publishing part of dcmjs if you want to look at a recent example github action.

Interesting, had not heard of semantic release before. @fedorov is this something we also want for libdicomicc or will we keep it at manual releases (for now)?

I really have zero experience, I am fine either way. The easiest sounds good to me! All I can add is that this publish workflow we use in dicom-microscopy-viewer succeeded as recently as Jan 15, 2026, and it does not use semantic release: https://github.com/ImagingDataCommons/dicom-microscopy-viewer/blob/master/.github/workflows/publish.yml.

fedorov · 2026-02-02T18:35:19Z

@rflamand I heard back from @hackermd - he is in the process of recovering his access to npm and setting up 2FA... I am not sure we can do much until that is sorted out.

rflamand · 2026-02-03T09:45:51Z

@rflamand I heard back from @hackermd - he is in the process of recovering his access to npm and setting up 2FA... I am not sure we can do much until that is sorted out.

No worries, thanks for looking into it!

Flamand, Raf and others added 5 commits December 11, 2025 09:17

Test simplified run_unit_test workflow

9ff246b

Small update to run_unit_tests and created publish actions

50f7977

Added caching + some guard rails

1d82175

Supply emcmake with caching folder. Should speed up runner

2a30285

Update version in build instructions

9734a86

fedorov requested a review from Copilot January 27, 2026 16:01

Copilot started reviewing on behalf of fedorov January 27, 2026 16:01 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

Process copilot comments

fb2b473

fedorov requested a review from Copilot January 28, 2026 11:46

Copilot started reviewing on behalf of fedorov January 28, 2026 11:47 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

rflamand added 5 commits January 28, 2026 13:07

Remove trailing spaces + apply some other formatting

bab342b

Add cmake to installed dependencies

56a2585

Remove yarn caching + add version verification before publishing

6b3f286

Update step names to remove yarn cache

8cc02ba

Remove yarn caching from ubuntu steps in run_unit_tests

837c59a

fedorov requested a review from Copilot January 28, 2026 14:17

Copilot started reviewing on behalf of fedorov January 28, 2026 14:17 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

	sudo apt-get install -y liblcms2-dev autoconf
	sudo apt-get install -y liblcms2-dev autoconf cmake

Conversation

rflamand commented Dec 11, 2025 • edited by fedorov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rflamand commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedorov commented Jan 27, 2026

Uh oh!

rflamand commented Jan 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

rflamand Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

rflamand Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

fedorov commented Jan 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rflamand commented Jan 28, 2026

Uh oh!

pieper commented Jan 28, 2026

Uh oh!

rflamand commented Jan 28, 2026

Uh oh!

fedorov commented Jan 28, 2026

Uh oh!

fedorov commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fedorov commented Feb 2, 2026

rflamand commented Dec 11, 2025 •

edited by fedorov

Loading

fedorov commented Jan 28, 2026 •

edited

Loading