-
Notifications
You must be signed in to change notification settings - Fork 65
CI: Support for pull-reviews video #949
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
825e9e9
78bb8af
bfa5573
5e7af87
804abbc
ee63099
c94d971
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||||
| name: Video Review | ||||||||||||||
|
|
||||||||||||||
| on: | ||||||||||||||
| pull_request: | ||||||||||||||
| types: [opened, synchronize] | ||||||||||||||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential duplicate review workflows. This workflow triggers on the same events ( Consider whether both workflows are needed, or if they should be consolidated or conditionally triggered. 🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| permissions: | ||||||||||||||
| contents: read | ||||||||||||||
| pull-requests: write | ||||||||||||||
| id-token: write | ||||||||||||||
|
|
||||||||||||||
| jobs: | ||||||||||||||
| preel: | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Job name appears to be a typo. The job name - preel:
+ pr-review:📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| uses: ambient-code/pull-reviews/.github/workflows/review.yml@main | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Pin external workflow to a specific commit SHA. Referencing the external workflow with - uses: ambient-code/pull-reviews/.github/workflows/review.yml@main
+ uses: ambient-code/pull-reviews/.github/workflows/review.yml@<commit-sha>🤖 Prompt for AI Agents |
||||||||||||||
| with: | ||||||||||||||
| s3_bucket: pull-reviews | ||||||||||||||
| gcp_project_id: ambient-code-platform | ||||||||||||||
| gcp_region: us-east5 | ||||||||||||||
| gcp_workload_identity_provider: projects/888214980327/locations/global/workloadIdentityPools/github/providers/ambient-code-github-actions | ||||||||||||||
|
Comment on lines
+17
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid hardcoded GCP environment identifiers in workflow inputs. Lines 17–19 hardcode project/region/provider values. This reduces portability across environments and exposes infra identifiers in repo config. Prefer Suggested refactor- gcp_project_id: ambient-code-platform
- gcp_region: us-east5
- gcp_workload_identity_provider: projects/888214980327/locations/global/workloadIdentityPools/github/providers/ambient-code-github-actions
+ gcp_project_id: ${{ secrets.GCP_PROJECT }}
+ gcp_region: ${{ secrets.GCP_REGION }}
+ gcp_workload_identity_provider: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| secrets: | ||||||||||||||
| S3_ENDPOINT: ${{ secrets.S3_ENDPOINT }} | ||||||||||||||
| S3_ACCESS_KEY_ID: ${{ secrets.S3_ACCESS_KEY_ID }} | ||||||||||||||
| S3_SECRET_ACCESS_KEY: ${{ secrets.S3_SECRET_ACCESS_KEY }} | ||||||||||||||
| CDN_BASE_URL: ${{ secrets.CDN_BASE_URL }} | ||||||||||||||
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.
Workflow name appears incorrect.
The name "Video Review" seems inconsistent with the PR's purpose of enabling automated pull request reviews. Consider renaming to something like "Pull Review" or "PR Review".
📝 Committable suggestion
🤖 Prompt for AI Agents