organization web hooks #1884
organization web hooks #1884
Conversation
Added the organization webhooks and tests. Corrected a few repository url (related to hooks). Added a ensure condition for hookid in repository hooks
2. Added EnsuresNonEmptyArguments() in OrganizationHooksClientTest.cs 3. Fixed the integrationTests I added for org webhooks 4. Changes asked by ryanribble
…leOrganizationHooksClient.cs
Hooks -> Hook fixtures-> Fixtures variable name changes in OrganizationsHooksFixture.cs
|
Sorry for the delay @MartinDawson, I'll take a look at reviewing this, this week |
|
It's been approximately 52 weeks, could this be reviewed? I have a strong need for it right now rather than the raw parsing I'm doing. |
@cliffchapmanrbx are you able to provide feedback for the changes? The more eyes we have on this, the quicker it should be when I can make time for it... |
I lack the internal familiarity to provide more than a "yep this is C# alright" overview of the code. If that would be sufficient for you I can gloss over it to look for egregious performance issues. Honestly I don't feel comfortable providing any feedback that would be taken at face value for this project at this time. If reviewers is something you need to get this moving forward I might be able to devote time to it. |
This would be a great help. We have a decent suite of automation and tests to catch things, but additional eyes from the "yep, this code makes sense" or "this bit of code looks weird" perspective would help. You shouldn't need to be deeply familiar with the internals to help out, but feel free to ask questions in here if there's something that doesn't make sense - I'm more than happy to turn these questions and answers into guidance for future reviews. |
Codecov Report
@@ Coverage Diff @@
## master #1884 +/- ##
==========================================
- Coverage 65.85% 65.07% -0.79%
==========================================
Files 547 553 +6
Lines 14301 14399 +98
Branches 838 836 -2
==========================================
- Hits 9418 9370 -48
- Misses 4724 4755 +31
- Partials 159 274 +115
|
There was a problem hiding this comment.
After giving this a look over I think I'm just going to push this out with the next update because I can't spot any obvious concerns.
Thanks for the contribution @MartinDawson!
|
release_notes: Add support for Organization webhooks |
nickfloyd
added
Type: Feature
#1028
This is a continuation and fix for PR #1144
@ryangribble I fixed all tests apart from these two:
@StanleyGoldman
These seem related to creating new releases?
Would be great to get some reviews on this soon as I really want to get this in.