refactor!: Update package types to align with webhook event #3515

adrian-saunders · 2025-03-13T18:24:06Z

BREAKING CHANGE: PackageVersion.Body and PackageVersion.Metadata are both now json.RawMessage.

Hello,

Firstly, thank you for maintaining this project! It has made interacting with GitHub significantly easier. I know this is a little backward to raise a PR without even a comment on the original issue so sorry but I had to get some legal approval first. Anyway...

This PR is meant to better align the package types with the webhook event schema.

Fixes: #2033

Breaking changes:

PackageVersion.Body is now json.RawMessage - I'm not sure in what scenarios this is a string or object but it can be both.
PackageVersion.Metadata is now json.json.RawMessage - webhooks return an array here, rather than a PackageMetadata

I've added getters for both of these fields.

References:
Webhooks: https://github.com/octokit/webhooks/tree/main/payload-schemas/api.github.com/package
REST: https://docs.github.com/en/rest/packages/packages?apiVersion=2022-11-28

Please do let me know your comments and if there is anything I haven't thought of!

Update the types associated with GitHub packages to better align with the schema used by by the package event.

codecov · 2025-03-13T18:42:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.30%. Comparing base (d0976cc) to head (14a2d8c).
Report is 13 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3515      +/-   ##
==========================================
+ Coverage   91.17%   91.30%   +0.12%     
==========================================
  Files         181      183       +2     
  Lines       15859    16094     +235     
==========================================
+ Hits        14460    14695     +235     
  Misses       1225     1225              
  Partials      174      174

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gmlewis · 2025-03-13T19:00:31Z

If you run ./script/lint.sh locally you should see the issues that need to be fixed. Thank you, @adrian-saunders!

gmlewis

Thank you, @adrian-saunders!
I'll continue my review after you address the current issues.

github/packages.go

gmlewis · 2025-03-13T19:10:53Z

github/packages.go

@@ -85,7 +157,6 @@ type PackageFile struct {
 	MD5         *string    `json:"md5,omitempty"`
 	ContentType *string    `json:"content_type,omitempty"`
 	State       *string    `json:"state,omitempty"`
-	Author      *User      `json:"author,omitempty"`


Why is this field being removed?

As far as I can tell, PackageFiles only exist on a webhook event and the schema doesn't have it as a property

https://github.com/octokit/webhooks/blob/main/payload-schemas/api.github.com/package/published.schema.json#L227-L238

Should have mentioned as a breaking change though!

OK, let's please leave the field in unless we can conclusively prove that it is no longer being used. Feel free to add a comment about your findings.

OK will add back

github/packages.go

gmlewis · 2025-03-14T00:46:13Z

Oh, sorry, I see your responses now in email.
I'll continue the review.

gmlewis

Thank you, @adrian-saunders!
It is unfortunate that we have so many any, map and json.RawMessage in this PR, but it sounds like it is pretty unavoidable due to the wide variety of possible data.

Let's please add back in the removed Author field, and then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

github/packages_test.go

adrian-saunders · 2025-03-14T11:01:53Z

github/packages.go

@@ -85,7 +157,6 @@ type PackageFile struct {
 	MD5         *string    `json:"md5,omitempty"`
 	ContentType *string    `json:"content_type,omitempty"`
 	State       *string    `json:"state,omitempty"`
-	Author      *User      `json:"author,omitempty"`


OK will add back

gmlewis

Thank you, @adrian-saunders!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@stevehipwell - might you have time for a code review? Thank you!

stevehipwell · 2025-03-14T19:12:18Z

@gmlewis I think this is another case where it's worth considering if json.RawMessage is the right solution or if we need an alternative such as to widen the struct field (e.g.single to array) or add a second field (object or string depending on the response)?

gmlewis · 2025-03-14T20:04:18Z

@gmlewis I think this is another case where it's worth considering if json.RawMessage is the right solution or if we need an alternative such as to widen the struct field (e.g.single to array) or add a second field (object or string depending on the response)?

I think that is certainly a possibility. Are there examples of what you are proposing elsewhere in this repo that can be used as a guide for implementation?

If not, I think it may be too much to ask PR authors to try and understand what you are envisioning, as my personal understanding is a bit fuzzy and I would not be able to give guidance.

Maybe the best way to go about this is to open one or more new Issues with enhancement proposals to make the developer experience better for users of this client library by implementing what you are proposing. In fact, maybe we could start with one or two specific API collections (maybe the two you've already mentioned... this PR and the other one you commented on) and see if we want to expand this idea throughout the repo based on how the changes are received?

stevehipwell

LGTM

gmlewis · 2025-03-17T17:58:28Z

Thank you, @stevehipwell!
Merging.

adrian-saunders · 2025-03-17T19:05:32Z

Thank you both for the review!

adrian-saunders added 2 commits March 8, 2025 17:51

Refactor package types

1c4f2be

Update the types associated with GitHub packages to better align with the schema used by by the package event.

update DockerMetadata and RubyMetadata to pointers

f9b5bf6

gmlewis added NeedsReview Breaking API Change labels Mar 13, 2025

gmlewis changed the title ~~refactor!: update package types to align with webhook event~~ refactor!: Update package types to align with webhook event Mar 13, 2025

gmlewis mentioned this pull request Mar 13, 2025

package event outdated #2033

Closed

gmlewis requested changes Mar 13, 2025

View reviewed changes

adrian-saunders added 3 commits March 13, 2025 23:42

pr comments

975bc19

update tests and run generate script

fa615bf

run linter

eb83767

adrian-saunders requested a review from gmlewis March 14, 2025 00:21

gmlewis requested changes Mar 14, 2025

View reviewed changes

adrian-saunders commented Mar 14, 2025

View reviewed changes

add back author field

14a2d8c

adrian-saunders requested a review from gmlewis March 14, 2025 15:26

gmlewis approved these changes Mar 14, 2025

View reviewed changes

stevehipwell approved these changes Mar 17, 2025

View reviewed changes

gmlewis removed the NeedsReview label Mar 17, 2025

gmlewis merged commit 7a89ae9 into google:master Mar 17, 2025
7 checks passed

gmlewis mentioned this pull request Mar 17, 2025

Bump version of go-github to v70.0.0 #3520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: Update package types to align with webhook event #3515

refactor!: Update package types to align with webhook event #3515

adrian-saunders commented Mar 13, 2025 •

edited by gmlewis

Loading

codecov bot commented Mar 13, 2025 •

edited

Loading

gmlewis commented Mar 13, 2025

gmlewis left a comment

gmlewis Mar 13, 2025

adrian-saunders Mar 14, 2025

gmlewis Mar 14, 2025

adrian-saunders Mar 14, 2025

gmlewis commented Mar 14, 2025

gmlewis left a comment •

edited

Loading

adrian-saunders Mar 14, 2025

gmlewis left a comment

stevehipwell commented Mar 14, 2025

gmlewis commented Mar 14, 2025 •

edited

Loading

stevehipwell left a comment

gmlewis commented Mar 17, 2025

adrian-saunders commented Mar 17, 2025

refactor!: Update package types to align with webhook event #3515

refactor!: Update package types to align with webhook event #3515

Conversation

adrian-saunders commented Mar 13, 2025 • edited by gmlewis Loading

codecov bot commented Mar 13, 2025 • edited Loading

Codecov Report

gmlewis commented Mar 13, 2025

gmlewis left a comment

Choose a reason for hiding this comment

gmlewis Mar 13, 2025

Choose a reason for hiding this comment

adrian-saunders Mar 14, 2025

Choose a reason for hiding this comment

gmlewis Mar 14, 2025

Choose a reason for hiding this comment

adrian-saunders Mar 14, 2025

Choose a reason for hiding this comment

gmlewis commented Mar 14, 2025

gmlewis left a comment • edited Loading

Choose a reason for hiding this comment

adrian-saunders Mar 14, 2025

Choose a reason for hiding this comment

gmlewis left a comment

Choose a reason for hiding this comment

stevehipwell commented Mar 14, 2025

gmlewis commented Mar 14, 2025 • edited Loading

stevehipwell left a comment

Choose a reason for hiding this comment

gmlewis commented Mar 17, 2025

adrian-saunders commented Mar 17, 2025

adrian-saunders commented Mar 13, 2025 •

edited by gmlewis

Loading

codecov bot commented Mar 13, 2025 •

edited

Loading

gmlewis left a comment •

edited

Loading

gmlewis commented Mar 14, 2025 •

edited

Loading