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 new method to access check run API #45

Merged
merged 4 commits into from
Dec 7, 2018

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Dec 4, 2018

Fix #34

No idea how to test this. And even if this add_check works, need to discuss how to actually use it, as it provides a lot more options than status checks.

TODO

  • Discuss how to use this.
  • Update blueprints to use this. @astrofrog will do this part.
  • Add tests. Cannot find a good way to test just the new method with mock and still have the test be useful.
  • Add change log.

NOTES

  • App owner needs to give app itself checks read/write access.
  • Repo owner where the app is installed needs to update app installation permission as well.

PROOF OF CONCEPT

pllim/playpen#19 (Ops, the status check area disappeared when I closed the PR. But note that the checks also appear as statuses at the bottom of the PR like a regular status. Like Tom R said, checks are like fancy statuses.)

@pllim pllim added the enhancement New feature or request label Dec 4, 2018
@pllim pllim requested a review from astrofrog December 4, 2018 07:24
@codecov-io
Copy link

codecov-io commented Dec 4, 2018

Codecov Report

Merging #45 into master will decrease coverage by 1.5%.
The diff coverage is 6.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   85.93%   84.43%   -1.51%     
==========================================
  Files          14       14              
  Lines         775      790      +15     
==========================================
+ Hits          666      667       +1     
- Misses        109      123      +14
Impacted Files Coverage Δ
baldrick/github/github_api.py 71.29% <6.66%> (-3.14%) ⬇️

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 88b21a8...3868a22. Read the comment docs.

@pllim pllim mentioned this pull request Dec 7, 2018
@pllim pllim changed the title [WIP] Add new method to access check run API Add new method to access check run API Dec 7, 2018
@pllim
Copy link
Contributor Author

pllim commented Dec 7, 2018

Other than foregoing the test, I think this is ready to go, @astrofrog . Perhaps this will be tested indirectly when you test a blueprint that uses this method?

Copy link
Collaborator

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Minor request but looks good otherwise!

baldrick/github/github_api.py Outdated Show resolved Hide resolved
@astrofrog astrofrog merged commit b0daa56 into OpenAstronomy:master Dec 7, 2018
@pllim pllim deleted the checks-api branch December 7, 2018 23:57
@pllim pllim mentioned this pull request May 1, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for using the new checks API
3 participants