-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 Base, Ref, and SHA to EditChange struct #1619
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @JacobValdemar .
Please add a unit test that exercises the marshal/unmarshal of this struct.
Co-authored-by: Glenn Lewis <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1619 +/- ##
=======================================
Coverage 68.06% 68.06%
=======================================
Files 97 97
Lines 8855 8855
=======================================
Hits 6027 6027
Misses 1912 1912
Partials 916 916
Continue to review full report at Codecov.
|
Good idea about the unit test @gmlewis. I can't locate where the other content in event_types.go are tested. Do you have any immediate position on where this test should be placed? 🙂 |
Oh, wow! You are right... I thought we had that. How about creating a new file, Thank you, @JacobValdemar ! |
Hi again @gmlewis :) I've now added the unit tests you requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @JacobValdemar !
LGTM.
Awaiting second LGTM before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @JacobValdemar
LGTM 👌🏼
Thank you, @wesleimp ! |
I'm wondering whether there is an estimated time of arrival of the feature in this PR to be shipped into a release. If I'm not wrong release Some downstream repositories (cross-referencing srvaroa/labeler#13) would benefit from having it on a release. Thanks. |
Hi @jhlegarreta - as of #1280, all releases (major, minor, and patch) are now on-demand. (I should probably put this in the README.md... sorry.) I won't be able to start the process quite yet, but I believe we could cut a new release later today (or early tomorrow) if I can get some assistance with code reviews. |
@gmlewis thanks for the prompt answer. Wow, it's nice that you are able to cut releases on demand. I appreciate the effort. |
Hi @jhlegarreta - https://github.com/google/go-github/releases/tag/v34.0.0 has been released. |
Much appreciated @gmlewis ! Thanks. If I am allowed to post here a question tangentially related to this PR (let me know if I should ask it elsewhere), I'd like to know whether the package supports Am I right or is the webhook supported using some other mechanism or path that I do not see? Would it be possible to add it as time permits? A few prominent open source projects from Apache or the ITK ecosystem could possibly benefit from it. Thanks. |
Ah, yes, it looks like our repo doesn't support it yet. I'll open an issue and invite volunteers to implement it. Thanks, @jhlegarreta . |
There is now a new issue to support it: #1834. |
Add support for webhook which is triggered when you change the base branch in a pull request after opening it.
Fixes: #1617