Fabisev/artifact publishing #149
Conversation
| @@ -0,0 +1,135 @@ | |||
| name: Build and Release | |||
There was a problem hiding this comment.
I see the general flow of this is different from what we did
- We are not using npmrc
- We are not specific scope modifier
--access=public
| if [[ "${{ steps.version.outputs.is_rc }}" == "true" ]]; then | ||
| npm publish aws-lambda-ric-*.tgz --tag rc | ||
| else | ||
| npm publish aws-lambda-ric-*.tgz |
There was a problem hiding this comment.
We publish for 2 arch -> We cannot use 1 tar file for it
scripts/add-headers.js
Outdated
| @@ -0,0 +1,113 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
For adding things like Copyright header the most classic tool is git diff - It's market practise - Let's reuse that and not language specific tools - Good thought of adding this as part of linting, I hat when build/automated review fail due to this:P!
scripts/generate-changelog.js
Outdated
| @@ -0,0 +1,97 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Why do we need this if we are specifying executable before starting script
scripts/test_local.sh
Outdated
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| set -e |
There was a problem hiding this comment.
For ease of debugging on this failing we can optionally do
set -euo pipefail
trap 'echo "Error on line $LINENO"; exit 1' ERR
scripts/test_local.sh
Outdated
| local node_version=$1 | ||
| echo "Running unit tests for Node.js $node_version..." | ||
| docker build -f test/unit/Dockerfile.nodejs${node_version}.x -t unit/nodejs.${node_version}x . | ||
| docker run --rm unit/nodejs.${node_version}x |
There was a problem hiding this comment.
For checking failures
docker run … || { echo "Tests failed for Node.js $node_version"; exit 1; }
| @@ -0,0 +1,36 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
In general for formatting bash doesn't care but we need to add formatting for ease in readability
scripts/test_local.sh
Outdated
| "all") | ||
| echo "Running unit tests for all Node versions..." | ||
| for version in 18 20 22; do | ||
| run_unit_tests $version |
There was a problem hiding this comment.
nit: Consider running them concurrently like command & => Only downside logs are interleaved which will make debugging hard
fabisev
force-pushed
the
fabisev/artifact-publishing
branch
7 times, most recently
from
8d292b5 to
2e64bbd
Compare
fabisev
force-pushed
the
fabisev/artifact-publishing
branch
from
2e64bbd to
bdb6bf1
Compare
fabisev
force-pushed
the
fabisev/artifact-publishing
branch
from
d0e6613 to
523de7c
Compare
fabisev
force-pushed
the
fabisev/artifact-publishing
branch
from
b3dfd05 to
160d714
Compare
scripts/add-headers.sh
Outdated
| first_line=$(head -n1 "$file" 2>/dev/null || echo "") | ||
|
|
||
| # Create patch entry | ||
| echo "--- a/$file" >> "$patch_file" |
There was a problem hiding this comment.
Patch file itself could be created beforehand?
scripts/check-headers.sh
Outdated
| fi | ||
|
|
||
| # Create patch entry | ||
| echo "--- a/$file" >> "$patch_file" |
There was a problem hiding this comment.
Same question here, we don't need to create the patch file on the fly
fabisev
force-pushed
the
fabisev/artifact-publishing
branch
from
3471e1c to
595e7e8
Compare
Issue #, if available:
Description of changes:
Target (OCI, Managed Runtime, both):
both
Checklist
npm installto generate thepackage-lock.jsoncorrectly and included it in the PR.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.