Skip to content
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

Merged
merged 10 commits into from Dec 14, 2022

Conversation

wolfy1339
Copy link
Member

@wolfy1339 wolfy1339 commented Dec 11, 2022

octokit/webhooks.js#646 will be resolved once the updates are merged there

ref: octokit/webhooks.net#178


Behavior

Before the change?

  • The registry_package event was missing
  • The package event was missing some information in the schemas

After the change?

Other information


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Feature/model/API additions: Type: Feature

@wolfy1339 wolfy1339 added the Type: Feature New feature or request label Dec 11, 2022
@ghost ghost added this to Features in JS Dec 11, 2022
@@ -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"],
Copy link
Member Author

@wolfy1339 wolfy1339 Dec 11, 2022

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

Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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.

@wolfy1339 wolfy1339 changed the title feat: add schemas for registry_package event, update package event with new information 🚧 feat: add schemas for registry_package event, update package event with new information Dec 11, 2022
@wolfy1339 wolfy1339 changed the title 🚧 feat: add schemas for registry_package event, update package event with new information feat: add schemas for registry_package event, update package event with new information Dec 11, 2022
@wolfy1339 wolfy1339 mentioned this pull request Dec 11, 2022
13 tasks
Copy link
Member

@kfcampbell kfcampbell left a 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.

@wolfy1339 wolfy1339 merged commit b7b5d9d into master Dec 14, 2022
JS automation moved this from Features to Done Dec 14, 2022
@wolfy1339 wolfy1339 deleted the update-package/registry-package branch December 14, 2022 20:25
@octokitbot
Copy link
Collaborator

🎉 This PR is included in version 6.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Type: Feature New feature or request
Projects
Archived in project
JS
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants