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 ListCheckRunsOptions with new field AppID #2236

Merged
merged 4 commits into from Jan 2, 2022
Merged

Update ListCheckRunsOptions with new field AppID #2236

merged 4 commits into from Jan 2, 2022

Conversation

rojanDinc
Copy link
Contributor

@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #2236 (1fecc18) into master (1d9884e) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2236      +/-   ##
==========================================
+ Coverage   97.79%   97.80%   +0.01%     
==========================================
  Files         113      113              
  Lines       10156    10205      +49     
==========================================
+ Hits         9932     9981      +49     
  Misses        156      156              
  Partials       68       68              
Impacted Files Coverage Δ
github/checks.go 100.00% <ø> (ø)
github/actions_workflow_runs.go 100.00% <0.00%> (ø)
github/teams.go 98.14% <0.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d9884e...1fecc18. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @rojanDinc!
Just one change, please, then we should be ready for a second LGTM before merging.

Note that any other contributor to this repo is welcome to provide the second LGTM/approval after reviewing.

github/checks.go Outdated Show resolved Hide resolved
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @rojanDinc !
LGTM.
Merging after tests pass.

@gmlewis
Copy link
Collaborator

gmlewis commented Dec 25, 2021

It looks like the tests failed. Please make sure to run go generate ./... and push the results.
Also make sure your code is run through go fmt to make the linter happy.
See CONTRIBUTING.md for more info.

@rojanDinc
Copy link
Contributor Author

Hi, I forgot to run tests locally once I changed AppIDs type to int64 but that problem should be resolved now.

github/checks.go Outdated
@@ -234,6 +234,7 @@ type ListCheckRunsOptions struct {
CheckName *string `url:"check_name,omitempty"` // Returns check runs with the specified name.
Status *string `url:"status,omitempty"` // Returns check runs with the specified status. Can be one of "queued", "in_progress", or "completed".
Filter *string `url:"filter,omitempty"` // Filters check runs by their completed_at timestamp. Can be one of "latest" (returning the most recent check runs) or "all". Default: "latest"
AppID *int64 `url:"app_id,omitempty"` // Filters check runs by GitHub App ID.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please run gofmt on this file and push the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. I've formatted the code hopefully everything is in place now.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @rojanDinc !
LGTM.
Merging.

@gmlewis gmlewis merged commit 504516e into google:master Jan 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants