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

ci: compile for distribution, remove node_modules #32

Merged
merged 1 commit into from Jun 3, 2022
Merged

ci: compile for distribution, remove node_modules #32

merged 1 commit into from Jun 3, 2022

Conversation

mchenryc
Copy link
Contributor

@mchenryc mchenryc commented Jun 1, 2022

This PR removes the unnecessary node_modules dir from the repository, instead providing an npm script to build a single .js file for distribution, as recommended for GitHub .js actions.

Changes after this will will need to be built (npm run build), and the generated files (in dist) added to the commit. This is so it can be used by other GitHub projects.

I added a test job to verify that the dist files are built and committed with each change - this just builds the PR, and ensures the results are the same as in the commit. It would be good to require the build status check to pass before merging (see GitHub Docs about status checks). This will avoid confusion between a forgetting to build and commit the distribution, and the test cucumber-report checks that intentionally "fail".

image.

Note that updating dependencies (and sometimes just running npm install locally) will update package-lock.json which also needs to be committed - this ensures the CI build uses the same dependency versions as were used to build the committed distribution locally.

Of the workflow jobs, only the test job installs node modules, to use cucumber. All tests still use dist/index.js to run tests, and the direct-from-repository-test job ensures the action works as is, without having to install node modules.

* Use ncc to create `dist/index.js`, as recommended in:
  https://docs.github.com/en/actions/creating-actions/creating-a-javascript-action#commit-tag-and-push-your-action-to-github
* Remove `node_modules` from repo
* Point action.yml at `dist/index.js`
* Add a job to `workflows/test.yml` to ensure `dist/index.js` is updated.
* Remove unused `express` and `mocha` dependencies
@deblockt
Copy link
Owner

deblockt commented Jun 3, 2022

Hi, Thank for this contribution 👍
I started actions, but it seem that there is an issue, I will try to understand what is going on:

HttpError: Resource not accessible by integration
    at /home/runner/work/cucumber-report-annotations-action/cucumber-report-annotations-action/webpack:/cucumber-report-annotations-action/node_modules/@octokit/request/dist-node/index.js:86:1
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at /home/runner/work/cucumber-report-annotations-action/cucumber-report-annotations-action/webpack:/cucumber-report-annotations-action/index.js:[17](https://github.com/deblockt/cucumber-report-annotations-action/runs/6721338625?check_suite_focus=true#step:3:18)0:1

@deblockt
Copy link
Owner

deblockt commented Jun 3, 2022

Ho I see that is a permission issue using forks. I will merge this PR.

Thanks a lot

@deblockt deblockt merged commit 60602f8 into deblockt:master Jun 3, 2022
@mchenryc mchenryc deleted the feat/compile-distribution branch June 15, 2022 12:26
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.

None yet

2 participants