-
Notifications
You must be signed in to change notification settings - Fork 3
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
Adding github workflows #11
Adding github workflows #11
Conversation
…h-protection-rules Fe 43 set up tests in cicd branch protection rules
Also successfully ran on this PR: https://github.com/great-expectations/jsonforms-antd-renderers/actions/runs/8089482156/job/22105431510?pr=11 |
.github/workflows/tests.yaml
Outdated
- name: Install Node.js | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: 18 |
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.
There's a way to reference the version specified in the .nvmrc--see mercury-web's deploy or validate workflows for an example of the exact syntax
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.
Can you point out where this is - I was not able to find it
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.
.github/workflows/tests.yaml
Outdated
${{ runner.os }}-pnpm-store- | ||
|
||
- name: Install dependencies | ||
run: pnpm install |
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.
Does this use the lockfile by default or do you need to specify frozen-lockfile?
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.
Updated this: https://pnpm.io/cli/install#--frozen-lockfile
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.
Are we planning to use the pre-commit stage? Was that added to our entire great-expectations organization or did we add that?
@NathanFarmer |
- main | ||
pull_request: | ||
branches: | ||
- main |
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 don't think this is an issue but want to double check: if joe schmoe opens a PR against this repo, can they do stuff like make changes to this workflow that logs api keys or anything like that? I'm pretty sure the answer is no
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.
It's not possible: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#redacting-secrets-from-workflow-run-logs
Also for secrets, forked repos do not have access to them
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Tested on my fork: https://github.com/TrangPham/jsonforms-antd-renderers/actions/runs/8089417491/job/22105275009