Dist gen generation#541
Open
tjuhaszrh wants to merge 9 commits into
Open
Conversation
Contributor
Author
|
[test] |
Testing Farm results
|
Contributor
Author
|
Tests seem to mostly pass, the failures look like infra issues. |
tjuhaszrh
force-pushed
the
dist-git2
branch
4 times, most recently
from
May 19, 2026 10:38
a5b6c0f to
6852710
Compare
Contributor
Author
|
[test] |
Contributor
Author
|
@phracek Could you do a review please? |
phracek
requested changes
Jun 4, 2026
phracek
left a comment
phracek
left a comment
Member
There was a problem hiding this comment.
Just a few comments. C8S is not supported at all.
Member
|
@tjuhaszrh Please update |
Contributor
Author
|
[test] |
Contributor
Author
|
I don't think the README check targets changes from this PR. |
Contributor
Author
Removed all mentions of c8s and regenerated files, so c8s dockerfiles should be gone. |
Member
|
@tjuhaszrh Please by |
Contributor
Author
|
[test] |
Member
|
[test] |
Member
|
[test-pytest] |
Contributor
Author
|
[test] |
phracek
requested changes
Jun 15, 2026
phracek
left a comment
phracek
left a comment
Member
There was a problem hiding this comment.
See my comments. Let's make the src/Dockerfile more readable. Let's remove a lot of duplicty.
But thanks for your good work.
| {% if spec.prod in ["c9s", "rhel10", "c10s"] %} | ||
| # Package libatomic_ops was removed | ||
| {% endif %}{% if spec.prod in ["rhel10", "c10s"] and spec.version == "22" %} | ||
| RUN INSTALL_PKGS="make gcc gcc-c++ git openssl-devel nodejs nodejs-nodemon nodejs-npm nss_wrapper-libs which" && \ |
Member
There was a problem hiding this comment.
what about to add none npm package to mamultispec.yml file and add only nodejs version or unversioned packages here.
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (45)
📝 WalkthroughWalkthroughThe change adds templated Node.js image sources, multispec build matrices, generation manifests, S2I lifecycle scripts, runtime helpers, and documentation. It also updates Node.js 22/24 metadata and generated artifacts, fills registry entries, and removes obsolete c8s Dockerfiles. ChangesNode.js image generation and runtime
Versioned artifact corrections
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant S2I
participant assemble
participant run
participant init-wrapper
participant NodeProcess
S2I->>assemble: provide application source
assemble->>assemble: configure npm and install dependencies
assemble->>assemble: build and prune production dependencies
S2I->>run: start container
run->>run: resolve DEV_MODE and startup command
run->>init-wrapper: launch wrapped command when enabled
init-wrapper->>NodeProcess: start and forward signals
run->>NodeProcess: launch direct or derived command
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/README.md (1)
5-5: ⚡ Quick winFix spelling and grammar errors in both README templates.
Multiple spelling errors and hyphenation issues reduce clarity and professional appearance. Consistent errors across both files:
src/README.md#L206, src/README.minimal.md#L278: "souce" → "source"src/README.md#L264, src/README.minimal.md#L336: "avalable" → "available"src/README.md#L149, src/README.minimal.md#L222: "environemnt variale" → "environment variable"src/README.md#L301, src/README.minimal.md#L372: "another versions" → "other versions"src/README.md#L5,L7, src/README.minimal.md#L7: "Fedora based" → "Fedora-based"src/README.md#L166, src/README.minimal.md#L238: "key value pairs" → "key-value pairs"🔧 Proposed fixes for spelling and grammar in src/README.md
-Users can choose between RHEL, CentOS and Fedora based images. +Users can choose between RHEL, CentOS and Fedora-based images. -and the Fedora images are available in [Quay.io](https://quay.io/organization/fedora). +and the Fedora-based images are available in [Quay.io](https://quay.io/organization/fedora). -In case of `NODE_CMD` environemnt variale is specified, then `init-wrapper` script will use the value of `NODE_CMD` to start your application. +In case of `NODE_CMD` environment variable is specified, then `init-wrapper` script will use the value of `NODE_CMD` to start your application. -One way to define a set of environment variables is to include them as key value pairs in your repo's `.s2i/environment` file. +One way to define a set of environment variables is to include them as key-value pairs in your repo's `.s2i/environment` file. -As part of development mode, this image supports hot deploy. If development mode is enabled, any souce code that is changed in the running container will be immediately reflected in the running nodejs application. +As part of development mode, this image supports hot deploy. If development mode is enabled, any source code that is changed in the running container will be immediately reflected in the running nodejs application. -A detailed explanation on how the init-wrapper script works is avalable in +A detailed explanation on how the init-wrapper script works is available in -In that repository you also can find another versions of Node.js environment Dockerfiles. +In that repository you also can find other versions of Node.js environment Dockerfiles.🔧 Proposed fixes for spelling and grammar in src/README.minimal.md
-Users can choose between RHEL, CentOS and Fedora based images. +Users can choose between RHEL, CentOS and Fedora-based images. -In case of `NODE_CMD` environemnt variale is specified, then `init-wrapper` script will use the value of `NODE_CMD` to start your application. +In case of `NODE_CMD` environment variable is specified, then `init-wrapper` script will use the value of `NODE_CMD` to start your application. -One way to define a set of environment variables is to include them as key value pairs in your repo's `.s2i/environment` file. +One way to define a set of environment variables is to include them as key-value pairs in your repo's `.s2i/environment` file. -As part of development mode, this image supports hot deploy. If development mode is enabled, any souce code that is changed in the running container will be immediately reflected in the running nodejs application. +As part of development mode, this image supports hot deploy. If development mode is enabled, any source code that is changed in the running container will be immediately reflected in the running nodejs application. -A detailed explanation on how the init-wrapper script works is avalable in +A detailed explanation on how the init-wrapper script works is available in -In that repository you also can find another versions of Node.js environment Dockerfiles. +In that repository you also can find other versions of Node.js environment Dockerfiles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/README.md` at line 5, Fix all spelling and grammar errors across src/README.md and src/README.minimal.md. In src/README.md at lines 5-7, change "Fedora based" to "Fedora-based". Additionally, in src/README.md, correct "souce" to "source" at line 206, "avalable" to "available" at line 264, "environemnt variale" to "environment variable" at line 149, "another versions" to "other versions" at line 301, and "key value pairs" to "key-value pairs" at line 166. In src/README.minimal.md, apply the same corrections at the corresponding locations: line 7 for "Fedora-based", line 278 for "source", line 336 for "available", line 222 for "environment variable", line 372 for "other versions", and line 238 for "key-value pairs".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Dockerfile`:
- Around line 71-73: The conditional guard on line 71 checks for
`config.post_install` but the RUN directive on line 73 renders
`spec.post_install`, causing a namespace mismatch. Since the post_install field
is defined in the spec namespace in specs/multispec.yml, the condition will
always evaluate to false and skip the post-install hook. Change the conditional
on line 71 from `{% if config.post_install %}` to `{% if spec.post_install %}`
to match the namespace being used in the RUN statement.
In `@src/s2i/bin/run`:
- Around line 31-37: The startup command detection has two issues: First, on
line 36, the condition `[ -n $package_json_main ]` uses an unquoted variable,
which when empty expands to `[ -n ]` and evaluates to true incorrectly—add
quotes around the variable to properly check if it is non-empty. Second, lines
31-32 execute sed commands on package.json without verifying the file exists
first, which will cause the script to exit under set -e if the file is
missing—add a file existence check before the sed commands that read the
package.json file to ensure the script handles missing package.json gracefully.
---
Nitpick comments:
In `@src/README.md`:
- Line 5: Fix all spelling and grammar errors across src/README.md and
src/README.minimal.md. In src/README.md at lines 5-7, change "Fedora based" to
"Fedora-based". Additionally, in src/README.md, correct "souce" to "source" at
line 206, "avalable" to "available" at line 264, "environemnt variale" to
"environment variable" at line 149, "another versions" to "other versions" at
line 301, and "key value pairs" to "key-value pairs" at line 166. In
src/README.minimal.md, apply the same corrections at the corresponding
locations: line 7 for "Fedora-based", line 278 for "source", line 336 for
"available", line 222 for "environment variable", line 372 for "other versions",
and line 238 for "key-value pairs".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48becdda-8e9b-440a-9d42-410371d84779
📒 Files selected for processing (45)
22-minimal/.exclude-c8s22-minimal/.exclude-c9s22-minimal/.exclude-fedora22-minimal/Dockerfile.c8s22-minimal/s2i/bin/init-wrapper22-minimal/s2i/bin/run22/.exclude-c8s22/.exclude-c9s22/.exclude-fedora22/Dockerfile.c8s22/Dockerfile.rhel822/Dockerfile.rhel922/s2i/bin/init-wrapper22/s2i/bin/run22/test24-minimal/.exclude-c8s24-minimal/.exclude-rhel824-minimal/Dockerfile.c8s24-minimal/s2i/bin/init-wrapper24-minimal/s2i/bin/run24/.exclude-c8s24/.exclude-rhel824/Dockerfile.c8s24/Dockerfile.rhel824/Dockerfile.rhel924/s2i/bin/init-wrapper24/s2i/bin/run24/testREADME.mdmanifest-minimal.ymlmanifest.ymlspecs/multispec.ymlsrc/Dockerfilesrc/README.mdsrc/README.minimal.mdsrc/root/opt/app-root/etc/generate_container_usersrc/root/opt/app-root/etc/npm_global_module_listsrc/root/opt/app-root/etc/scl_enablesrc/root/usr/bin/fix-permissionssrc/s2i/bin/assemblesrc/s2i/bin/init-wrappersrc/s2i/bin/runsrc/s2i/bin/run-minimalsrc/s2i/bin/save-artifactssrc/s2i/bin/usage
💤 Files with no reviewable changes (4)
- 24-minimal/Dockerfile.c8s
- 24/Dockerfile.c8s
- 22-minimal/Dockerfile.c8s
- 22/Dockerfile.c8s
Comment on lines
+31
to
+37
| package_json_start=$(sed -n 's/\s*"start"\s*:\s*"\(.*\)".*/\1/p' package.json) | ||
| package_json_main=$(sed -n 's/\s*"main"\s*:\s*"\(.*\)".*/\1/p' package.json) | ||
|
|
||
| if [ -n "$package_json_start" ]; then | ||
| start_command=$package_json_start | ||
| elif [ -n $package_json_main ]; then | ||
| start_command="node ." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
unset package_json_main
if [ -n $package_json_main ]; then
echo "unquoted check => true (bug)"
else
echo "unquoted check => false"
fi
if [ -n "$package_json_main" ]; then
echo "quoted check => true"
else
echo "quoted check => false (expected)"
fi
bash -c 'set -e; sed -n "s/.*/x/p" package.json; echo "still running"' 2>/dev/null || \
echo "missing package.json causes non-zero path under set -e"Repository: sclorg/s2i-nodejs-container
Length of output: 126
🏁 Script executed:
cat -n src/s2i/bin/run | head -50Repository: sclorg/s2i-nodejs-container
Length of output: 2147
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Testing unquoted [ -n \$var ] without set -u ==="
unset package_json_main
if [ -n $package_json_main ]; then
echo "unquoted empty var => true"
else
echo "unquoted empty var => false (correct)"
fi
echo ""
echo "=== Testing sed on missing file under set -e ==="
bash -c 'set -e; sed -n "s/.*/x/p" nonexistent.json; echo "still running"' 2>&1 || \
echo "FAILED: script exited due to sed failure (as expected)"
echo ""
echo "=== Verifying lines 34-37 logic ==="
# Simulate the scenario
package_json_start=""
package_json_main=""
echo "package_json_start='$package_json_start'"
echo "package_json_main='$package_json_main'"
if [ -n "$package_json_start" ]; then
echo "Result: uses package_json_start"
elif [ -n $package_json_main ]; then
echo "Result: uses package_json_main (node .)"
else
echo "Result: falls through to other checks"
fiRepository: sclorg/s2i-nodejs-container
Length of output: 435
Startup command detection fails when package_json_main is empty, and script exits if package.json is missing.
Line 36 uses [ -n $package_json_main ] unquoted. When the variable is empty, this expands to [ -n ], which evaluates to true because it tests whether the string "-n" itself is non-empty—incorrectly forcing start_command="node .". Additionally, lines 31–32 execute sed on package.json without checking file existence first, so a missing file will cause the script to exit under set -e.
Proposed fix
- package_json_start=$(sed -n 's/\s*"start"\s*:\s*"\(.*\)".*/\1/p' package.json)
- package_json_main=$(sed -n 's/\s*"main"\s*:\s*"\(.*\)".*/\1/p' package.json)
+ package_json_start=""
+ package_json_main=""
+ if [ -f "package.json" ]; then
+ package_json_start=$(sed -n 's/[[:space:]]*"start"[[:space:]]*:[[:space:]]*"\(.*\)".*/\1/p' package.json)
+ package_json_main=$(sed -n 's/[[:space:]]*"main"[[:space:]]*:[[:space:]]*"\(.*\)".*/\1/p' package.json)
+ fi
@@
- elif [ -n $package_json_main ]; then
+ elif [ -n "$package_json_main" ]; then
start_command="node ."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| package_json_start=$(sed -n 's/\s*"start"\s*:\s*"\(.*\)".*/\1/p' package.json) | |
| package_json_main=$(sed -n 's/\s*"main"\s*:\s*"\(.*\)".*/\1/p' package.json) | |
| if [ -n "$package_json_start" ]; then | |
| start_command=$package_json_start | |
| elif [ -n $package_json_main ]; then | |
| start_command="node ." | |
| package_json_start="" | |
| package_json_main="" | |
| if [ -f "package.json" ]; then | |
| package_json_start=$(sed -n 's/[[:space:]]*"start"[[:space:]]*:[[:space:]]*"\(.*\)".*/\1/p' package.json) | |
| package_json_main=$(sed -n 's/[[:space:]]*"main"[[:space:]]*:[[:space:]]*"\(.*\)".*/\1/p' package.json) | |
| fi | |
| if [ -n "$package_json_start" ]; then | |
| start_command=$package_json_start | |
| elif [ -n "$package_json_main" ]; then | |
| start_command="node ." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/s2i/bin/run` around lines 31 - 37, The startup command detection has two
issues: First, on line 36, the condition `[ -n $package_json_main ]` uses an
unquoted variable, which when empty expands to `[ -n ]` and evaluates to true
incorrectly—add quotes around the variable to properly check if it is non-empty.
Second, lines 31-32 execute sed commands on package.json without verifying the
file exists first, which will cause the script to exit under set -e if the file
is missing—add a file existence check before the sed commands that read the
package.json file to ensure the script handles missing package.json gracefully.
phracek
requested changes
Jun 17, 2026
phracek
left a comment
phracek
left a comment
Member
There was a problem hiding this comment.
Last round. Now it looks really awesome.
Contributor
Author
|
[test] |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/README.md (1)
29-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix Markdown lint violations in the source templates.
The same generated-formatting issues appear in both files.
src/README.md#L29-L34: add fence languages and normalize heading formatting throughout the template.src/README.minimal.md#L36-L61: apply the same changes, then regenerate artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/README.md` around lines 29 - 34, Fix Markdown linting in both source templates: update src/README.md lines 29-34 and src/README.minimal.md lines 36-61 by adding appropriate languages to fenced code blocks and normalizing heading formatting throughout each template, then regenerate the derived artifacts.Source: Linters/SAST tools
src/Dockerfile (1)
72-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify script path targeting.
Since
$STI_SCRIPTS_PATHcorrectly resolves to/usr/libexec/s2iin the utilized S2I core bases, you can simplify the template by referencing the environment variable unconditionally.♻️ Proposed refactor
-RUN chmod +x {% if spec.version == "20" %}$STI_SCRIPTS_PATH/init-wrapper{% else %}/usr/libexec/s2i/init-wrapper{% endif %} +RUN chmod +x $STI_SCRIPTS_PATH/init-wrapper🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Dockerfile` at line 72, Update the init-wrapper chmod command to use $STI_SCRIPTS_PATH/init-wrapper unconditionally, removing the spec.version conditional while preserving the existing executable-permission step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/multispec-npm-config.yml`:
- Around line 64-65: Update the full_image_name configuration for rhel10 to
append the :latest tag, matching the tag format used by the other image
configurations while leaving img_name unchanged.
In `@src/Dockerfile`:
- Around line 56-58: Update the conditional removal commands in the Node.js
symlink setup to use force removal, replacing each relevant rm invocation for
node, npm, and npx with rm -f so builds continue when the unversioned binaries
are absent.
- Around line 65-68: Move the spec.post_install hook into the preceding RUN
command before the package-manager clean all operation, preserving valid command
chaining and avoiding a standalone RUN block ending with && \. Remove the
separate RUN {{ spec.post_install }} block while keeping the existing
conditional behavior for configurations without post_install.
In `@src/README.md`:
- Around line 289-296: Update the custom-command examples in src/README.md lines
289-296 and src/README.minimal.md lines 361-368 to use INIT_WRAPPER=true in both
container build and container start commands, preserving the documented
init-wrapper usage.
- Around line 191-194: Correct both Podman development-mode examples by adding
the missing --env option before DEBUG_PORT=5454 in src/README.md lines 191-194
and src/README.minimal.md lines 263-266, ensuring both commands set DEBUG_PORT
correctly.
- Around line 227-250: Fix the package.json examples in src/README.md lines
227-250 and src/README.minimal.md lines 299-322 by removing the inline
annotations and trailing commas, or change both fenced blocks to jsonc
consistently so the examples are valid as documented.
---
Nitpick comments:
In `@src/Dockerfile`:
- Line 72: Update the init-wrapper chmod command to use
$STI_SCRIPTS_PATH/init-wrapper unconditionally, removing the spec.version
conditional while preserving the existing executable-permission step.
In `@src/README.md`:
- Around line 29-34: Fix Markdown linting in both source templates: update
src/README.md lines 29-34 and src/README.minimal.md lines 36-61 by adding
appropriate languages to fenced code blocks and normalizing heading formatting
throughout each template, then regenerate the derived artifacts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 326e8a59-ae2a-4781-97e8-e72da2fe3747
📒 Files selected for processing (45)
22-minimal/.exclude-c8s22-minimal/.exclude-c9s22-minimal/.exclude-fedora22-minimal/Dockerfile.c8s22-minimal/s2i/bin/init-wrapper22-minimal/s2i/bin/run22/.exclude-c8s22/.exclude-c9s22/.exclude-fedora22/Dockerfile.rhel822/Dockerfile.rhel922/s2i/bin/init-wrapper22/s2i/bin/run22/test24-minimal/.exclude-c8s24-minimal/.exclude-rhel824-minimal/Dockerfile.c8s24-minimal/s2i/bin/init-wrapper24-minimal/s2i/bin/run24/.exclude-c8s24/.exclude-rhel824/Dockerfile.c8s24/Dockerfile.rhel824/Dockerfile.rhel924/s2i/bin/init-wrapper24/s2i/bin/run24/testREADME.mdmanifest-minimal.ymlmanifest.ymlspecs/multispec-npm-config.ymlspecs/multispec.ymlsrc/Dockerfilesrc/README.mdsrc/README.minimal.mdsrc/root/opt/app-root/etc/generate_container_usersrc/root/opt/app-root/etc/npm_global_module_listsrc/root/opt/app-root/etc/scl_enablesrc/root/usr/bin/fix-permissionssrc/s2i/bin/assemblesrc/s2i/bin/init-wrappersrc/s2i/bin/runsrc/s2i/bin/run-minimalsrc/s2i/bin/save-artifactssrc/s2i/bin/usage
💤 Files with no reviewable changes (3)
- 24-minimal/Dockerfile.c8s
- 24/Dockerfile.c8s
- 22-minimal/Dockerfile.c8s
🚧 Files skipped from review as they are similar to previous changes (24)
- 24/test
- 24-minimal/s2i/bin/run
- 22/s2i/bin/run
- src/s2i/bin/init-wrapper
- 22/Dockerfile.rhel9
- src/s2i/bin/save-artifacts
- 22/Dockerfile.rhel8
- 22/test
- 24/Dockerfile.rhel9
- manifest.yml
- 24/Dockerfile.rhel8
- 22-minimal/s2i/bin/run
- src/root/opt/app-root/etc/npm_global_module_list
- src/root/opt/app-root/etc/scl_enable
- src/root/usr/bin/fix-permissions
- src/s2i/bin/usage
- src/s2i/bin/run
- README.md
- manifest-minimal.yml
- src/s2i/bin/assemble
- 24/s2i/bin/run
- src/s2i/bin/run-minimal
- specs/multispec.yml
- src/root/opt/app-root/etc/generate_container_user
Comment on lines
+64
to
+65
| img_name: "ubi10/$NAME-$NODEJS_VERSION" | ||
| full_image_name: "ubi10/$NAME-$NODEJS_VERSION" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing :latest tag to the full image name.
The full_image_name for rhel10 is missing the :latest tag, unlike rhel8, rhel9, c9s, and c10s. This causes a divergence in the generated usage label in the Dockerfile.
💚 Proposed fix
img_name: "ubi10/$NAME-$NODEJS_VERSION"
- full_image_name: "ubi10/$NAME-$NODEJS_VERSION"
+ full_image_name: "ubi10/$NAME-$NODEJS_VERSION:latest"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| img_name: "ubi10/$NAME-$NODEJS_VERSION" | |
| full_image_name: "ubi10/$NAME-$NODEJS_VERSION" | |
| img_name: "ubi10/$NAME-$NODEJS_VERSION" | |
| full_image_name: "ubi10/$NAME-$NODEJS_VERSION:latest" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/multispec-npm-config.yml` around lines 64 - 65, Update the
full_image_name configuration for rhel10 to append the :latest tag, matching the
tag format used by the other image configurations while leaving img_name
unchanged.
Comment on lines
+191
to
+194
| To run the container in development mode with a debug port of 5454, run: | ||
| ``` | ||
| $ podman run --env DEV_MODE=true DEBUG_PORT=5454 my-image-id | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the Podman environment-variable examples.
Both templates omit --env for DEBUG_PORT, so the documented command does not set the variable.
src/README.md#L191-L194: changeDEBUG_PORT=5454to--env DEBUG_PORT=5454.src/README.minimal.md#L263-L266: changeDEBUG_PORT=5454to--env DEBUG_PORT=5454.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 192-192: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 193-193: Dollar signs used before commands without showing output
(MD014, commands-show-output)
📍 Affects 2 files
src/README.md#L191-L194(this comment)src/README.minimal.md#L263-L266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/README.md` around lines 191 - 194, Correct both Podman development-mode
examples by adding the missing --env option before DEBUG_PORT=5454 in
src/README.md lines 191-194 and src/README.minimal.md lines 263-266, ensuring
both commands set DEBUG_PORT correctly.
Comment on lines
+227
to
+250
| ```json | ||
| { | ||
| "name": "node-echo", | ||
| "version": "0.0.1", | ||
| "description": "node-echo", | ||
| "main": "example.js", <--- main attribute | ||
| "dependencies": { | ||
| }, | ||
| "devDependencies": { | ||
| "nodemon": "*" | ||
| }, | ||
| "engine": { | ||
| "node": "*", | ||
| "npm": "*" | ||
| }, | ||
| "scripts": { | ||
| "dev": "nodemon --ignore node_modules/ server.js", | ||
| "start": "node server.js" <-- start script | ||
| }, | ||
| "keywords": [ | ||
| "Echo" | ||
| ], | ||
| "license": "", | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the invalid package.json examples.
Both fenced examples contain non-JSON annotations and trailing commas.
src/README.md#L227-L250: remove the inline annotations/trailing comma or mark the block asjsonc.src/README.minimal.md#L299-L322: apply the same correction.
📍 Affects 2 files
src/README.md#L227-L250(this comment)src/README.minimal.md#L299-L322
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/README.md` around lines 227 - 250, Fix the package.json examples in
src/README.md lines 227-250 and src/README.minimal.md lines 299-322 by removing
the inline annotations and trailing commas, or change both fenced blocks to
jsonc consistently so the examples are valid as documented.
- Clean, readable template using universal variables
- Uses {{ spec.build_deps }} and {{ spec.pkgs }} instead of complex conditionals
- MariaDB-style consolidated RUN instruction
- Removed redundant comment blocks for better readability
- Universal variables with readable multiline formatting - Version-specific pkgs and build_deps using clean Jinja2 conditionals - Eliminates need for complex logic in Dockerfile templates - Maintains all distro-specific package requirements
Remove c8s files and configurations from the project.
Regenerate README.md with updated version table.
Contributor
Author
|
[test] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleaning up the draft PR.
Summary by CodeRabbit