go-github
go-github copied to clipboard
Add enums for webhook payloads action field
Relevant Issue: https://github.com/google/go-github/issues/3127
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 92.88%. Comparing base (
2b8c7fa) to head (7fb9e96). Report is 233 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #3136 +/- ##
==========================================
- Coverage 97.72% 92.88% -4.84%
==========================================
Files 153 170 +17
Lines 13390 11426 -1964
==========================================
- Hits 13085 10613 -2472
- Misses 215 723 +508
Partials 90 90
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Ah! Now I understand better the scope of this breaking API change. I apologize that I couldn't quite visualize fully what we were discussing before.
I would like @willnorris to weigh in on this PR to get his thoughts on the value of this change vs the breaking nature of it (and therefore the impact on developers) before we proceed further.
Makes sense!
@gmlewis @willnorris any update on the PR?
this can be done progressively as well if you prefer and do it one event at a time (or in batches)
this can be done progressively as well if you prefer and do it one event at a time (or in batches)
If we're going to proceed, then I think one huge breaking change like this is probably better than a bunch of smaller changes.
But I would still like to hear what @willnorris thinks about changing all the *strings into explicit enum values.
@willnorris - do you have any opinions about a large breaking change like this to change a bunch of *strings into explicit enum values?
Would be interested to know if there is a decision taken about this. I personally would prefer enums as they are self documenting the possibilities a value can have.
@willnorris - do you have any opinions about a large breaking change like this to change a bunch of
*strings into explicit enum values?
Sorry I missed your question back in August. Personally, I'm not a fan. The surface area of the library is already huge, so I'd be pretty sensitive to changes like this that increase it all the more. 🤷🏻
@willnorris - do you have any opinions about a large breaking change like this to change a bunch of
*strings into explicit enum values?Sorry I missed your question back in August. Personally, I'm not a fan. The surface area of the library is already huge, so I'd be pretty sensitive to changes like this that increase it all the more. 🤷🏻
Thank you, @willnorris - and thank you, @prnvbn for your patience. I will close this PR as we will not be pursuing this change.