-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
Resolves: Add a CI/CD pipeline with GitHub Actions #1057
Resolves: Add a CI/CD pipeline with GitHub Actions #1057
Conversation
.github/workflows/ci-cd.yml
Outdated
name: "Logs ${{ matrix.os }} - nodejs ${{ matrix.node-version }}" | ||
path: logs/ | ||
|
||
deploy: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire step should be left out, I believe.
Terser is still published to NPM by hand, and moving to a flow like that should at least be a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @fabiosantoscode
to be sure I understood you correctly, you would like the build
and deploy
jobs to be on separate PRs ?
Something like this:
- PR-1
name: CI/CD pipeline
on: [ push, pull_request, workflow_dispatch ]
jobs:
build:
name: "Build ${{ matrix.os }} node${{ matrix.node-version }}"
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ ubuntu-latest, windows-latest ]
node-version: [ 10.x, 12.x, 14.x, 16.x ]
outputs:
package_version: ${{ steps.proj_ver_determiner.outputs.package_version }}
package_version_tag: ${{ steps.proj_ver_determiner.outputs.package_version_tag }}
latest_github_tag: ${{ steps.proj_ver_determiner.outputs.latest_github_tag }}
should_deploy: ${{ steps.proj_ver_determiner.outputs.should_deploy }}
steps:
...
- PR-2
+++
deploy:
if: needs.build.outputs.should_deploy == 'true'
needs: build
runs-on: ubuntu-latest
steps:
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there shouldn't be a deploy
step for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood.
We'll make the changes promptly 🙂
Hello there! I left just one comment, otherwise this looks good! |
Hey! Sorry I'm late! This looks good to go. Thanks! |
The pipeline runs:
build
job executes on every push commit, every PR and when the pipeline is run manuallydeploy
job executes only on a push commit to the default branch, that contains a new version in thepackage.json
NPM_TOKEN
GH_USER_EMAIL
andGH_USER_NAME
Please let us know if anything could be done better and/or if any other automation is required by the project! We would be happy to improve it to satisfactory completion 🙂
Resolves #1056