-
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
Change DependabotSecretsSelectedRepoIDs to []string #2401
Conversation
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@lneves75 - could you please review and approve if you agree to this change when you have a chance? |
Codecov Report
@@ Coverage Diff @@
## master #2401 +/- ##
=======================================
Coverage 98.06% 98.06%
=======================================
Files 119 119
Lines 10546 10546
=======================================
Hits 10342 10342
Misses 140 140
Partials 64 64
Continue to review full report at Codecov.
|
Awaiting LGTM+Approval from two contributors to this repo before merging. |
@gmlewis apparently the problem lies on github's API implementation for both endpoints. {
"message": "Invalid request.\n\nFor 'items', \"123\" is not an integer.\nFor 'items', \"456\" is not an integer.",
"documentation_url": "https://docs.github.com/rest/reference/actions#create-or-update-an-organization-secret"
} So I'm afraid that we need to have two distinct types 😞 type ActionSecretsSelectedRepoIDs []int64
type DependabotSecretsSelectedRepoIDs []string WDYT ? |
OK, that is really weird. |
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@lneves75 - I've updated the PR, if you could please try it out for correctness, that would be greatly appreciated! |
👋🏻 Hey there! I'm the PM for the API at GitHub, and I happened to just spot this thread. I can confirm that the current behaviour of the API is that:
Sorry for the pain! This is obviously a bit weird at the moment and is something we need to clean up - but in a way that minimises breaking changes for users. |
Hi @timrogers - it's great to have you here! Thank you to you and your team for an amazing product... it is greatly appreciated!
If I'm reading this correctly, then making them both use an array of strings should work. So then we tried splitting it into two separate implementations, as you see this PR now.
No problem... we would just like to get it working, so if you have recommendations on how to proceed, that would be great. |
@gmlewis So sorry - you’re right, I showed too much confidence and I actually misread the internal PR 😅: the Actions API really does only accept integers, whereas before it claimed incorrectly in the docs to accept strings. Looking at the inputs, integers seem like the right data type, so I will raise an issue with the Dependabot team to look at making this more consistent in a non-breaking way. Very glad I came by this repo to discover this and see all of the great work you’re doing to make the API accessible to Go developers ❤️ |
In other words, it looks like you’re doing exactly what you need to do in this PR! |
Thank you so much, @timrogers ! We really appreciate your input, and we always love to hear from GitHub team members and tech support whenever we can. 😄 |
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.
LGTM
Thank you, @raynigon ! |
Fixes: #2400.