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

Add InstallationsCount to App #2765

Merged
merged 1 commit into from Apr 19, 2023
Merged

Conversation

chmouel
Copy link
Contributor

@chmouel chmouel commented Apr 19, 2023

It's not in the official github documentation yet but exposed to the API.

% curl -s -H "Accept: application/vnd.github.v3+json" -H "Authorization: Bearer $T" https://api.github.com/app|grep installations_count
  "installations_count": 3

@chmouel chmouel force-pushed the addinstallcount branch 2 times, most recently from dbc9fff to 94c4092 Compare April 19, 2023 12:13
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, @chmouel .
Please run go generate ./... at the top of the repo and push the results to this PR.
See CONTRIBUTING.md for more details.

github/apps.go Outdated Show resolved Hide resolved
@gmlewis gmlewis changed the title Add InstallationsCount to appps.App Add InstallationsCount to apps.App Apr 19, 2023
@gmlewis gmlewis changed the title Add InstallationsCount to apps.App Add InstallationsCount to App Apr 19, 2023
@chmouel chmouel force-pushed the addinstallcount branch 2 times, most recently from 2b0710a to 876a434 Compare April 19, 2023 15:41
@gmlewis
Copy link
Collaborator

gmlewis commented Apr 19, 2023

Please see the other change I asked for above and also please make sure all unit tests pass before pushing (not force-pushing since we always squash-and-merge in this repo) changes to the PR.

@chmouel
Copy link
Contributor Author

chmouel commented Apr 19, 2023

@gmlewis apologies for the noise, i kinda messed my pr!

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 19, 2023

@gmlewis apologies for the noise, i kinda messed my pr!

No problem, @chmouel ! There's no rush... please feel free to take as long as you wish.

@chmouel
Copy link
Contributor Author

chmouel commented Apr 19, 2023

I am not really sure why it fails on CI as it works locally

% go clean -testcache
% go generate github.com/google/go-github/..
% go test github.com/google/go-github/...
?   	github.com/google/go-github/v51/test/fields	[no test files]
?   	github.com/google/go-github/v51/test/integration	[no test files]
ok  	github.com/google/go-github/v51/github	0.898s

is it because i am using go 1.20 ?

Signed-off-by: Chmouel Boudjnah <chmouel@chmouel.com>
@chmouel
Copy link
Contributor Author

chmouel commented Apr 19, 2023

ah it works better with go generate -x should we maybe update CONTRIBUTING.md ? or maybe was there any other caching somewhere on my system, because -x is only supposed to print...

@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #2765 (9d5f093) into master (693ddff) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2765   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files         132      132           
  Lines       11638    11638           
=======================================
  Hits        11412    11412           
  Misses        154      154           
  Partials       72       72           
Impacted Files Coverage Δ
github/apps.go 95.16% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 19, 2023

ah it works better with go generate -x should we maybe update CONTRIBUTING.md ?

Feel free to clarify if you think that would be helpful.

@chmouel
Copy link
Contributor Author

chmouel commented Apr 19, 2023

Feel free to clarify if you think that would be helpful.

I think it was not just my day today, and just was too quick without thinking much, apologies again for the noise, will be careful next time when contributing.

(I missed a dot to go generate github.com/google/go-github/.. 🤦🏻 )

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 19, 2023

No worries! Glad you got it all worked out. 😁

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, @chmouel !
LGTM.
Merging.

@gmlewis gmlewis merged commit 848d85f into google:master Apr 19, 2023
9 checks passed
@chmouel chmouel deleted the addinstallcount branch April 19, 2023 17:25
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