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

updating test action #131

Merged
merged 5 commits into from
Mar 5, 2024
Merged

updating test action #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e8e8c4a
updating test action
roodboi Mar 5, 2024
016e3c5
update
roodboi Mar 5, 2024
f120f14
whitespace
roodboi Mar 5, 2024
318c4fa
update event name
roodboi Mar 5, 2024
53c2ec5
dont check event name
roodboi Mar 5, 2024
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
21 changes: 15 additions & 6 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
name: Test
on:
pull_request_target:
branches: [ master ]
push:
branches:
- main
pull_request:
branches:
- main
jobs:
test-main:
if: github.ref == 'refs/heads/main'
if: github.repository == 'instructor-ai/instructor-js' && github.ref == 'refs/heads/main'
name: run-tests
runs-on: ubuntu-latest
environment: TEST-MAIN
Expand Down Expand Up @@ -39,10 +38,19 @@ jobs:
- run: bun run lint
- run: bun test --timeout=25000

approve:
if: github.ref != 'refs/heads/main'
Copy link
Contributor

Choose a reason for hiding this comment

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

The PR description mentions that the approve and test-branch jobs have been changed to trigger on pull_request instead of pull_request_target, but I don't see this change reflected in the diff. The condition for these jobs is github.ref != 'refs/heads/main', which means they will run for any branch that is not main, regardless of whether it's a pull request or not. Please confirm if this is an oversight.

runs-on: ubuntu-latest

steps:
- name: Approve
run: echo For security reasons, all pull requests need to be approved first before running any automated CI.

test-branch:
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'test-branch' job now runs on 'pull_request_target' and depends on the 'approve' job. As mentioned earlier, 'pull_request_target' could lead to testing stale code. Also, the dependency on 'approve' job might not add any value as the 'approve' job does not perform any approval action.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the approve job just logs out - but the repo settings require an approval from an admin to run

if: github.event_name == 'pull_request'
if: github.ref != 'refs/heads/main'
name: run-tests
runs-on: ubuntu-latest
needs: [approve]
environment: OPENAI
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
Expand All @@ -52,7 +60,8 @@ jobs:
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 2

- name: Setup bun
uses: oven-sh/setup-bun@v1
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,4 @@ This project is licensed under the terms of the MIT License.
- [ ] Action Item and Dependency Mapping

These translations provide a structured approach to creating TypeScript schemas with Zod, mirroring the functionality and intent of the original Python examples.