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.

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

Update Check Run HeadSHA #1652

Closed
Adirio opened this issue Nov 18, 2020 · 5 comments · Fixed by #1658
Closed

Update Check Run HeadSHA #1652

Adirio opened this issue Nov 18, 2020 · 5 comments · Fixed by #1658
Assignees
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). enhancement good first issue

Comments

@Adirio
Copy link

Adirio commented Nov 18, 2020

go-github/github/checks.go

Lines 175 to 186 in 1ff2714

// UpdateCheckRunOptions sets up parameters needed to update a CheckRun.
type UpdateCheckRunOptions struct {
Name string `json:"name"` // The name of the check (e.g., "code-coverage"). (Required.)
HeadSHA *string `json:"head_sha,omitempty"` // The SHA of the commit. (Optional.)
DetailsURL *string `json:"details_url,omitempty"` // The URL of the integrator's site that has the full details of the check. (Optional.)
ExternalID *string `json:"external_id,omitempty"` // A reference for the run on the integrator's system. (Optional.)
Status *string `json:"status,omitempty"` // The current status. Can be one of "queued", "in_progress", or "completed". Default: "queued". (Optional.)
Conclusion *string `json:"conclusion,omitempty"` // Can be one of "success", "failure", "neutral", "cancelled", "skipped", "timed_out", or "action_required". (Optional. Required if you provide a status of "completed".)
CompletedAt *Timestamp `json:"completed_at,omitempty"` // The time the check completed. (Optional. Required if you provide conclusion.)
Output *CheckRunOutput `json:"output,omitempty"` // Provide descriptive details about the run. (Optional)
Actions []*CheckRunAction `json:"actions,omitempty"` // Possible further actions the integrator can perform, which a user may trigger. (Optional.)
}

Despite having an optional HeadSHA field, if you update this to a different commit hash it won't be updated.

We have some Check Runs that are PR-oriented and not commit-oriented and we were trying to use this field to rebind the Check Runs to the head commit of a PR on synchronize.

Is this a bug?

Adirio added a commit to Adirio/kubebuilder-release-tools that referenced this issue Nov 18, 2020
Related: google/go-github#1652
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
Adirio added a commit to Adirio/kubebuilder-release-tools that referenced this issue Nov 18, 2020
Related: google/go-github#1652
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
@gmlewis
Copy link
Collaborator

gmlewis commented Nov 18, 2020

As far as I can tell from the official documentation: https://docs.github.com/en/free-pro-team@latest/rest/reference/checks#update-a-check-run
the head_sha field is no longer accepted as a parameter and is completely ignored.
(Therefore, we probably ought to remove this field from the struct entirely.)

You may wish to contact GitHub technical support and ask them the same question.

@Adirio
Copy link
Author

Adirio commented Nov 18, 2020

+1 to removing HeadSHA from the UpdateCheckRunOptions struct to prevent further confussion.

Adirio added a commit to Adirio/kubebuilder-release-tools that referenced this issue Nov 18, 2020
Related: google/go-github#1652
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
@gmlewis
Copy link
Collaborator

gmlewis commented Nov 18, 2020

The other fields should be verified as well as part of addressing this issue.

This would be a great PR for any new contributor to this repo or a new Go developer.
All contributions are greatly appreciated!

Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work.

Please check out our CONTRIBUTING.md guide to get started.

Thank you!

@ishanupadhyay
Copy link
Contributor

ishanupadhyay commented Dec 2, 2020

Hi @gmlewis @Adirio I can work on this issue. Can anyone of you please assign it to me?

@ishanupadhyay
Copy link
Contributor

ishanupadhyay commented Dec 2, 2020

Please take a look at PR-1657 . It will be my first contribution to this project :)

@gmlewis gmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). enhancement good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants