-
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
runnerID and runnerGroupID are int64, not string #2127
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Thank you, @sfunkhouser - can you find any official GitHub docs that show these values as integers? I'm curious because I thought the original implementation came from an example where they were strings... but now I'm having troubles finding the examples. Also, if you could please fix the CLA and the unit tests, we can move forward with this PR. |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
Codecov Report
@@ Coverage Diff @@
## master #2127 +/- ##
=======================================
Coverage 97.78% 97.78%
=======================================
Files 112 112
Lines 9970 9970
=======================================
Hits 9749 9749
Misses 154 154
Partials 67 67
Continue to review full report at Codecov.
|
Looking for the docs, I've only found the runner docs, which reference them as integers (https://docs.github.com/en/enterprise-server@2.22/rest/reference/enterprise-admin#add-a-self-hosted-runner-to-a-group-for-an-enterprise) and when I ran through the audit logs I got an unmarshal error but let me see if I can find some more concrete documentation on this. I'll follow-up tomorrow. |
Ah, yes! There it is. Thank you for the pointer! So I'm not sure what happened, but this should fix it. Thanks again, @sfunkhouser ! |
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, @sfunkhouser !
LGTM.
Merging.
When running this before the change, results that included runnerID or runnerGroupID could not be parsed. This PR changes the type for both from
string
->int64