-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(github): use deployment statuses #5733
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
Conversation
Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
|
Thanks @mmiszy, this looks good. Can you share how one can set the deployment, possibly via a That would make it easier for me to test (without having to dig through GitHub's API). |
|
@erezrokah deployments can be created with: curl \
-u {username}:{oauthToken} \
-X POST \
-H "Accept: application/vnd.github.v3+json" \
https://api.github.com/repos/{owner}/{repo}/deployments \
-d '{ "ref":"main", "required_contexts":[], "environment": "test", "description": "Some description" }'and then status can be added via: curl \
-u {username}:{oauthToken} \
-X POST \
-H "Accept: application/vnd.github.v3+json" \
https://api.github.com/repos/{owner}/{repo}/deployments/{deployment_id}/statuses \
-d '{ "state": "success", "target_url": "https://google.com" }' |
erezrokah
left a comment
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.
Thanks @mmiszy, sorry for the delay to review.
I added a comment with 2 questions about the code.
Please let me know what you think
| : PreviewState.Other, | ||
| })); | ||
|
|
||
| return statuses.reverse().find(s => s.state === PreviewState.Success) || statuses[0]; |
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 can return undefined in case there are no statuses for the deployment.
Then the resulting array would be depStatuses = [undefined].
Should we add a .filter(Boolean) on the result?
Also it seems this tries to get the last successful status, or the first status if there are no successful ones.
This doesn't match the GraphQL query that always returns the last status. Is that intended?
Update: another issue I noticed is the statuses API returns the newest status first, so the reverse is redundant. Can you confirm this?
Bumping this |
|
Closing as stale per #5733 (comment). Please comment if you'd like to re-open |
|
This pr is definitely needed in order to support Vercel deployment URLs! |
|
Hello, Due to AWS Amplify working with the checks API and netlify working with the status API - I would appreciate an out-of-the-box ability for netlify to resolve the preview links against the checks API. Just wanted to give my input based on erezrokah's request to comment if I'd like to re-open. |
Reopening #4368
Fixes #4359
See #4368 for more details. This PR includes fixed tests and Prettier formatting.