Updated git data commit response with signature verification object #1398
Conversation
| @@ -1,4 +1,6 @@ | |||
| using System.Collections.Generic; | |||
| using Octokit.Internal; | |||
There was a problem hiding this comment.
💄 reorder these so System.* is before the rest...
|
@Sarmad93 looking good, just a few little things to clean up. Love that integration test 😍 |
|
👍 for review. Fixing this. |
| @@ -27,7 +27,9 @@ public static class AcceptHeaders | |||
| public const string MigrationsApiPreview = "application/vnd.github.wyandotte-preview+json"; | |||
|
|
|||
| public const string ReactionsPreview = "application/vnd.github.squirrel-girl-preview"; | |||
|
|
|||
|
|
|||
| public const string SignatureVerification = "application/vnd.github.cryptographer-preview+sha"; | |||
There was a problem hiding this comment.
Maybe name this SignatureVerificationPreview just for consistency with all the other preview headers?
|
Right..@ryangribble thanks for the review. 😄 |
| using System.Globalization; | ||
| using Octokit.Internal; | ||
|
|
||
| namespace Octokit.Models.Response |
There was a problem hiding this comment.
There's a coding convention here that I guess needs to be documented properly. Despite these files living in Octokit/Models/Responses the corresponding namespace for these is just Octokit. This means new using statements are unnecessary for consumers (and for this PR).
|
@Sarmad93 @ryangribble thanks for waiting for me. Just that one comment and I think this is good to go! |
| @@ -1,5 +1,6 @@ | |||
| using System.Threading.Tasks; | |||
| using Octokit; | |||
| using Octokit.Models.Response; | |||
There was a problem hiding this comment.
As mentioned elsewhere, this should be unnecessary after renaming the namespace...
yeh no worries. i already knew your absence when i saw your tweet about your off for a week 😉 |
|
@Sarmad93 looks good, thanks for this!
|
|
|
Fixes #1254
Hi @ryangribble @shiftkey could you plz review this code 😃 Regarding Integration test i am little confused where should i put this method, is it fine here in
CommitsClientTests? And what do you think of this integration test, right now this test is passing on repository which is of my test account. Is it fine here to hard code(string literals) repository parameter? Your feedback on this is much needed so that we can make improvement to this.