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.

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

Add Node Actions Dist workflow #14

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add Node Actions Dist workflow #14

wants to merge 2 commits into from

Conversation

GeekMasher
Copy link
Contributor

Key changes:

  • .github/workflows/actions-dist-node.yml: Added a new GitHub Actions workflow named "Actions Dist - Node". This workflow is triggered on a workflow call and has two jobs:
    • dist-check: This job checks for uncommitted changes in the dist folder for pull requests. It runs on ubuntu-latest, checks out the repository, sets up Node.js, installs and builds the project, and then checks for uncommitted changes.
    • dist-dependabot: This job updates the dist folder for dependabot commits in the specified branch. It runs on ubuntu-latest, checks out the repository, sets up Node.js, installs and builds the project, checks that the build is clean, and if not, updates the release files.

@GeekMasher GeekMasher requested a review from peter-murray April 12, 2024 12:39
- name: "Install and Build"
run: |
npm install
npm run bundle --if-present

Choose a reason for hiding this comment

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

Maybe introduce the script names of bundle and bundle-exe as inputs that are defaulted, this is not a standard naming convention

npm run bundle-exe --if-present

- name: "Check for uncommitted changes"
id: diff

Choose a reason for hiding this comment

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

Why does this have an id, it is not being referenced anywhere

uses: actions/checkout@v4

- name: "Setup Node"
uses: actions/setup-node@v2

Choose a reason for hiding this comment

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

v4 as above

default: "main"

jobs:
dist-check:

Choose a reason for hiding this comment

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

This job does not depend on the other, is that intentional? If so it is kind of doing what the other one is with a little more, so why the two?


- name: "Install and Build"
run: |
npm install

Choose a reason for hiding this comment

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

npm ci is probably the one you want here unless you want to update packages, ci will strictly follow the lock file and fail if there is a dependency not in it that would be installed

git config user.name github-actions
git config user.email [email protected]

git add .

Choose a reason for hiding this comment

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

If you know this to be the dist directory or something the user can specify, maybe lock it to that and then ensure that there are no extra changes outside of that, as that is sign something "additional" happened that was not expected?

@GeekMasher GeekMasher self-assigned this May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants