-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: add schemas for registry_package
event, update package
event with new information
#747
Conversation
@@ -33,138 +33,221 @@ | |||
"ecosystem": { "type": "string" }, | |||
"package_type": { | |||
"type": "string", | |||
"enum": ["npm", "maven", "rubygems", "docker", "nuget", "container"], | |||
"enum": ["npm", "maven", "rubygems", "docker", "nuget", "CONTAINER"], |
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.
Can someone verify if all the other values are also in caps? The values aren't listed in the OpenAPI schema
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.
Where are you getting the caps from? I'm looking internally at some GitHub code and I see container is lower-cased, as are the rest of the values.
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.
It's the payload examples in octokit/webhooks.net#178
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.
Oh I see! That's interesting; I haven't found any instances internally that use the capitalized form.
registry_package
event, update package
event with new informationregistry_package
event, update package
event with new information
registry_package
event, update package
event with new informationregistry_package
event, update package
event with new information
payload-schemas/api.github.com/common/package-npm-metadata.schema.json
Outdated
Show resolved
Hide resolved
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.
This looks good to me! The capitalization thing with CONTAINER is a little weird but I don't think it's a blocker.
🎉 This PR is included in version 6.8.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
octokit/webhooks.js#646 will be resolved once the updates are merged there
ref: octokit/webhooks.net#178
Behavior
Before the change?
registry_package
event was missingpackage
event was missing some information in the schemasAfter the change?
registry_package
event is now presentpackage
event now validates properly against a recent payload (Payloads taken from [BUG]: Exceptions thrown when processingpackage
events webhooks.net#178 and have been anonymized)Other information
d25cb45
, and from the payloadsAdditional info
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!
Type: Breaking change
label)If
Yes
, what's the impact:Pull request type
Please add the corresponding label for change this PR introduces:
Type: Feature