Skip to content

GDL-9 publish npm package to registry #206

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.

Sign up for GitHub

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

Open
tcarmet wants to merge 1 commit into
base: development/7.10
Choose a base branch
from
+128 −23
Open

GDL-9 publish npm package to registry #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
name: Release
Comment thread
tcarmet marked this conversation as resolved.

run-name: release ${{ inputs.tag }}

on:
workflow_dispatch:
Comment thread
tcarmet marked this conversation as resolved.
inputs:
tag:
description: 'Tag to be released'
required: true

jobs:
publish:
permissions:
packages: write
id-token: write
contents: read
env:
VERSION: ${{ inputs.tag }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set env vars
run: |
echo PKG_VERSION="$(jq -r .version < package.json)" >> $GITHUB_ENV
echo PKG_NAME="$(jq -r .name < package.json)" >> $GITHUB_ENV
echo PKG_BASENAME="$(basename $(jq -r .name < package.json))" >> $GITHUB_ENV
- name: Ensure the branch is protected
run: |
if [[ "${{ github.ref_protected }}" = "false" ]]; then
echo "::error::The branch ${{ github.ref }} is not protected"
exit 1
fi
- name: Ensure tag has not been released
run: |
TAG_EXISTS=$(git tag -l "${{ inputs.tag }}")
if [[ -n "$TAG_EXISTS" ]]; then
echo "::error::The tag ${{ inputs.tag }} already exists"
exit 1
fi
- name: Ensure version is properly set
run: |
if ! [[ "$PKG_VERSION" = "$VERSION" ]]; then
echo "::error file=package.json,line=$(sed -n '/"version":/=' package.json)::The tag $VERSION must match the version $PKG_VERSION specified in package.json"
exit 1
fi
- name: Setup node with GitHub Packages
uses: actions/setup-node@v4
with:
cache: yarn
node-version: '16'
registry-url: https://npm.pkg.github.com
Comment thread
tcarmet marked this conversation as resolved.
- name: install dependencies
run: yarn install --frozen-lockfile
- name: Publish to GitHub Packages
run: npm publish --provenance
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Setup node with npmjs.org
uses: actions/setup-node@v4
with:
registry-url: https://registry.npmjs.org
- name: Publish to npmjs.org
run: npm publish --provenance
Copy link
Copy Markdown
Contributor

@francoisferrand francoisferrand Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what happens if the push to npmjs fails, i.e. how can be "finisih" the release?

  • there is a "tag" check, which prevents generating the tag if already done : OK since the tag (and release) is created last
  • when publish to Github packages fails, nothing was actually changed : OK
  • when publish to npmjs fails, github package has already been published:
    • re-running may fail (?) as we already pushed to github?
    • if we re-run the release, we may have a different commit on the "new" release.... and could end-up with an incorrect build on github?
    • in the mean-time, the release is not present, but the package is already in github and may be consummed/upgraded unexpectedly... should we remove it? or make the tag/release first (but this adds complexity to re-run)?
    • as the tag is not created, bert-e (when used) may also mark more jira tickets as "fixed" in this release... this is not a new issue, but agravated if the release takes longer...

Copy link
Copy Markdown
Contributor Author

@tcarmet tcarmet Feb 19, 2025

Choose a reason for hiding this comment

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

You wanna change the order, this way if it fails on npm it didn't published on GitHub packages?

Also let's try not to overthink this too much, let's get some field experience first.

Copy link
Copy Markdown
Contributor

@francoisferrand francoisferrand Feb 21, 2025

Choose a reason for hiding this comment

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

I don't want to change the order, I want to merge something where we have a path, in case of failure : because such failures do and will happen, and typically when it happens it is at the worst of times...

So it is really not overthinking, but actually just thinking it through, to try and have some way to finish the release anyway when we need it.

Copy link
Copy Markdown
Contributor Author

@tcarmet tcarmet Feb 21, 2025

Choose a reason for hiding this comment

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

I see, ran a couple of test and confirm that we will face some failure if for example we want to re-publish a package to GitHub (I guess the same will happen on npm):

npm error code E409
npm error 409 Conflict - PUT https://npm.pkg.github.com/@scality%2feslint-config - Cannot publish over existing version
npm error A complete log of this run can be found in: /home/codespace/.npm/_logs/2025-02-21T21_38_24_404Z-debug-0.log

Now, my main concern here, is not towards what you want to apply, it's mostly that we are adding too much code/logic into something that as far as we established will be copy pasted.

So if we are duplicating code, let's keep it "dumb" even if the feature are limited. But if we want to add more checks and all, I rather we develop an action and reuse that code, otherwise it will be a pain to maintain across 20 different repos.

Let me know which path you prefer, either are fine by me I don't mind developing an action for it.

Copy link
Copy Markdown
Contributor

@francoisferrand francoisferrand Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

sorry for the delay, this happened before holiday, and missed it when coming back...

the point is not to try and overthink it, but really just to be prepared for failure: i.e. be sure we now how we can we "fix" the release when it fails (and be able to do it, even when it fails at worst of time)... Some procedure could be acceptable (though it may need permissions, like to delete the release from NPM and/or GH Packages)

I am guessing the simplest way to do this is to split this into 4 jobs, so they may be retried:

  • Pre-check & building (yarn install)
  • Upload to GH packages registry
  • Upload to NPM registry
  • Make the GH release

Notes:

  • this is actually what we already do in many release jobs (where we push docker images or ISOs)
  • this allows to handle more cleanly the registry-url override
  • uploads to GH and NPM will be done in parallel
  • not sure "when" in the jobs graph the release should be made: before upload makes it consistent with the tag check/... (and bert-e), but prevents having links to the packages in the release... Maybe the proper approach is to push the git tag after build (for consistency), then upload, and finish the release -with existing tag- when all uploads succeed

Copy link
Copy Markdown
Contributor Author

@tcarmet tcarmet Jun 23, 2025

Choose a reason for hiding this comment

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

No worries for the delay, down to split the jobs, that's actually how I originally proposed this PR if I remember correctly, but was redirected to a single job under the argument that we are building/installing the package twice and not technically publishing the same package.

I honestly think it's fine, and the benefit of having this capability to retry outweighs the fact that we are building twice. But stating it here just in case we don't want that.

Copy link
Copy Markdown
Contributor

@francoisferrand francoisferrand Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think installing and building should be done just once (esp. since it can be flaky, and may break cache), but this can be done by storing an artifact:

build:
   - git checkout
   - yarn install
   - yarn build
   - yarn test
   - upload-artifact "*.*"
publish_npm:
   - download-artifact
   - yarn publish
publish_github:
   - download-artifact
   - yarn publish

esp. if/when we retry publishing, we don't want to actually rebuild and risk having something different...

env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
- name: Create Release
uses: softprops/action-gh-release@v2
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
tag_name: ${{ inputs.tag }}
name: Release ${{ inputs.tag }}
target_commitish: ${{ github.sha }}
Comment thread
tcarmet marked this conversation as resolved.
append_body: |
Github Packages: https://github.com/${{ github.repository }}/pkgs/npm/${{ env.PKG_BASENAME }}
npmjs: https://www.npmjs.com/package/${{ env.PKG_NAME }}/v/${{ env.PKG_VERSION }}
43 changes: 31 additions & 12 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,39 @@ name: tests
on:
push:
branches-ignore:
- 'development/**'
Comment thread
tcarmet marked this conversation as resolved.
- q/*/*

jobs:
test:
permissions:
packages: write
id-token: write
contents: read
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: '16'
- name: install dependencies
run: yarn install --frozen-lockfile
- name: lint markdown
run: yarn lint_md
- name: lint javascript
run: yarn lint
- name: Checkout
uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: '16'
registry-url: https://npm.pkg.github.com
- name: install dependencies
run: yarn install --frozen-lockfile
- name: lint markdown
run: yarn lint_md
- name: lint javascript
run: yarn lint
- name: Set common env vars for npm publish
run: |
echo "SHORTSHA=$(git rev-parse --short $GITHUB_SHA)" >> $GITHUB_ENV
echo "PKG_VERSION=$(jq -r .version < package.json)" >> $GITHUB_ENV
- name: Set version for work in progress
run: yarn version --no-git-tag-version --new-version "0.0.0-sha.${SHORTSHA}"
if: github.ref_protected == false
- name: Set version for protected branch
run: yarn version --no-git-tag-version --new-version "${PKG_VERSION}-sha.${SHORTSHA}"
if: github.ref_protected
- name: Publish development version
run: npm publish --provenance --tag "${{ github.ref_name }}"
env:
NODE_AUTH_TOKEN: ${{ github.token }}
28 changes: 17 additions & 11 deletions package.json
View file
Open in desktop
Comment thread
tcarmet marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,43 +1,49 @@
{
"name": "eslint-config-scality",
"name": "@scality/eslint-config",
"version": "7.10.2",
"description": "ESLint config for Scality's Node.js coding guidelines",
"bin": {
"mdlint": "./bin/mdlint.js"
"mdlint": "bin/mdlint.js"
},
"main": "index.js",
"scripts": {
"test": "npm run --silent lint && npm run --silent lint_md",
"lint": "node_modules/.bin/eslint -c index.js $(git ls-files '*.js')",
"lint": "eslint -c index.js $(git ls-files '*.js')",
"lint_md": "node bin/mdlint.js $(git ls-files '*.md')"
},
"repository": {
"type": "git",
"url": "https://github.com/scality/Guidelines"
"url": "git+https://github.com/scality/Guidelines.git"
},
"dependencies": {
"commander": "1.3.2",
"markdownlint": "^0.25.1"
},
"bugs": {
"url": "https://github.com/scality/Guidelines/issues"
},
"devDependencies": {
"babel-eslint": "6.1.2",
"eslint": "^8.7.0",
"eslint-config-airbnb": "6.2.0"
},
"keywords": [
"eslint",
"eslintconfig",
"config",
"airbnb",
"scality",
"javascript",
"markdown",
"scality",
"styleguide"
],
"author": "Giorgio Regni",
"author": "Scality Object Squad <object-squad@scality.com>",
"license": "Apache-2.0",
"homepage": "https://github.com/scality/Guidelines",
"engines": {
"node": ">=16"
}
},
"publishConfig": {
"access": "public"
},
"files": [
"bin",
"*.[c]js"
]
}