Skip to content
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

chore: fix npm-publish dependencies and add provenance #3099

Merged
merged 1 commit into from
Dec 2, 2024
Merged

chore: fix npm-publish dependencies and add provenance #3099

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
35 changes: 15 additions & 20 deletions .github/workflows/release-please.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,45 +4,40 @@ on:
push:
branches:
- main
pull_request:

jobs:
release-please:
outputs:
pr: ${{ steps.release.outputs.pr }}
release_created: ${{ steps.release.outputs.release_created }}
permissions:
contents: write # to create release commit (googleapis/release-please-action)
pull-requests: write # to create release PR (googleapis/release-please-action)
if: github.event_name == 'push'
runs-on: ubuntu-latest
steps:
- uses: googleapis/release-please-action@v4
id: release
# Standard Conventional Commits: `feat` and `fix`
# node-gyp subdirectories: `bin`, `gyp`, `lib`, `src`, `test`
# node-gyp subcommands: `build`, `clean`, `configure`, `install`, `list`, `rebuild`, `remove`
# Core abstract category: `deps`
# Languages/platforms: `python`, `lin`, `linux`, `mac`, `macos`, `win`, `window`, `zos`
# Documentation: `doc`, `docs`, `readme`
# Standard Conventional Commits: `chore` (under "Miscellaneous")
# Miscellaneous abstract categories: `refactor`, `ci`, `meta`

test:
name: Release Test
needs: [ release-please ]
if: needs.release-please.outputs.pr || startsWith(github.head_ref, 'release-please--')
uses: ./.github/workflows/tests.yml
- uses: googleapis/release-please-action@v4
id: release
# Standard Conventional Commits: `feat` and `fix`
# node-gyp subdirectories: `bin`, `gyp`, `lib`, `src`, `test`
# node-gyp subcommands: `build`, `clean`, `configure`, `install`, `list`, `rebuild`, `remove`
# Core abstract category: `deps`
# Languages/platforms: `python`, `lin`, `linux`, `mac`, `macos`, `win`, `window`, `zos`
# Documentation: `doc`, `docs`, `readme`
# Standard Conventional Commits: `chore` (under "Miscellaneous")
# Miscellaneous abstract categories: `refactor`, `ci`, `meta`

npm-publish:
needs: release-please
if: ${{ needs.release-please.outputs.release_created }}
runs-on: ubuntu-latest
permissions:
contents: read
id-token: write # to generate npm provenance statements
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: lts/*
registry-url: 'https://registry.npmjs.org'
- run: npm publish --access public
- run: npm publish --provenance --access public
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
3 changes: 2 additions & 1 deletion .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jobs:
steps:
- uses: actions/checkout@v4
- run: pip install --user ruff
- run: ruff check --output-format=github --select="E,F,PLC,PLE,UP,W,YTT" --ignore="E721,PLC1901,S101,UP031" --target-version=py38 .
# Excluding `/gyp` directory as it is been checked in https://github.com/nodejs/gyp-next/ already
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumption does not make me comfortable. When you vendor-in source code, it becomes your code, and you should run your own tests on it.

Copy link
Member

Choose a reason for hiding this comment

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

Can we run two separate checks? It would be great to surface issues in /gyp because we do vendor it. But in the past it has created friction because a linting error in /gyp code would fail all of CI.

Copy link
Member

Choose a reason for hiding this comment

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

@legendecas Can we revert this part and re-open as a new PR? It would be nice to unblock the release from CI and figure out python linting in a separate discussion.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is added because the ruff check is failing in this repo: https://github.com/nodejs/node-gyp/actions/runs/12087642318/job/33709509643. But the ruff check is not failing on the gyp-next repo.

There are differences on the ruff configurations between the two repos. I'm fine with reverting this one but we will have to merge this PR with failing CIs.

Copy link
Member

@lukekarrys lukekarrys Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the explanation @legendecas, I didn't realize CI was failing. I think this change should be landed to make CI green and a new issue/PR opened to get two different ruff checks running, each with the correct configuration.

- run: ruff check --output-format=github --extend-exclude=gyp --select="E,F,PLC,PLE,UP,W,YTT" --ignore="E721,PLC1901,S101,UP031" --target-version=py38 .

lint-js:
name: Lint JS
Expand Down
Loading