-
Notifications
You must be signed in to change notification settings - Fork 50
ci: fix registry notification workflow (PLT-846) #331
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,31 +1,26 @@ | ||
| # runs if /packages/registry/** is updated and the release workflow completes successfully | ||
| # Notify the registry API after the release workflow completes successfully. | ||
| name: Update Registry Submodules | ||
|
|
||
| on: | ||
| workflow_run: | ||
| workflows: ["Release"] | ||
| types: | ||
| - completed | ||
| push: | ||
| paths: | ||
| - 'packages/registry/**' | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| jobs: | ||
| release: | ||
| notify: | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success' | ||
| if: github.event.workflow_run.conclusion == 'success' | ||
|
|
||
| steps: | ||
| - name: Check out code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 # Fetch the full history instead of a shallow clone | ||
|
|
||
| - name: Delay for 90 seconds to allow NPM package to propagate | ||
| - name: Delay for 90 seconds to allow npm package to propagate | ||
| run: sleep 90 | ||
|
|
||
| - name: Update registry API | ||
| uses: peter-evans/repository-dispatch@v1 | ||
| uses: peter-evans/repository-dispatch@v4 | ||
|
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. [suggestion] The v1→v4 bump itself is safe — Worth confirming while you're in here, though, whether this step actually notifies anything today. Two things stack up:
Both predate this PR, so not a blocker on the diff. But given the PR title is "fix registry notification workflow," it seems worth checking that the registry API is really receiving these — otherwise the fix is polishing a no-op. |
||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| event-type: workflow_completed | ||
|
|
||
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.
[nit]
contents: writeis the correct minimum for the repository-dispatch REST endpoint — good call declaring it explicitly. Since there's only one job, you could scope it underjobs.notify.permissionsinstead, so any future job added to this file starts from the default read-only token rather than inheriting write.