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

Implement Deployment Statuses Preview #1895

Merged
merged 8 commits into from
Feb 25, 2020

Conversation

@ryangribble
Copy link
Contributor

Hey @Cyberboss sorry I haven't reviewed yet, I'll take a look this week 👍

Copy link
Contributor

@ryangribble ryangribble left a comment

Choose a reason for hiding this comment

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

This looks great @Cyberboss

Just one minor request around the unit tests, to fix up the previous usage of the AcceptHeader const/variable, in favour of hardcoding the expected header string like the other tests already do 👍

Have you run any integration tests or confirmed that these changes do actually work?

Octokit.Tests/Clients/DeploymentStatusClientTests.cs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 9, 2020

Codecov Report

Merging #1895 into master will increase coverage by <.01%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #1895      +/-   ##
==========================================
+ Coverage   66.94%   66.95%   +<.01%     
==========================================
  Files         542      542              
  Lines       14223    14228       +5     
==========================================
+ Hits         9522     9526       +4     
- Misses       4701     4702       +1
Impacted Files Coverage Δ
Octokit/Helpers/AcceptHeaders.cs 100% <ø> (ø) ⬆️
Octokit/Models/Response/DeploymentStatus.cs 32.14% <ø> (ø) ⬆️
Octokit/Models/Request/NewDeploymentStatus.cs 30% <0%> (-3.34%) ⬇️
Octokit/Clients/DeploymentStatusClient.cs 100% <100%> (ø) ⬆️

@shiftkey shiftkey force-pushed the ImplementDeploymentEnums branch from c37e530 to b6ad71e Compare February 25, 2020 23:27
@shiftkey shiftkey self-assigned this Feb 25, 2020
@shiftkey
Copy link
Member

@ryangribble I added a new test to leverage the new properties, integration tests are passing locally

@shiftkey shiftkey merged commit b904ada into octokit:master Feb 25, 2020
@shiftkey
Copy link
Member

release_notes: Added support for Deployment Status API improvements

@Cyberboss Cyberboss deleted the ImplementDeploymentEnums branch February 29, 2020 18:41
@Cyberboss
Copy link
Contributor Author

Thanks for finishing this ancient thing off guys!

@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants