go-playground / webhooks

:fishing_pole_and_fish: Webhook receiver for GitHub, Bitbucket, GitLab, Gogs
MIT License
950 stars 238 forks source link

fix: Installation Struct Missing for CommitCommentPayload, IssueCommentPayload and IssuesPayload #178

Closed Fish-Nullify closed 1 year ago

Fish-Nullify commented 1 year ago

This pull request fixes issue https://github.com/go-playground/webhooks/issues/177

coveralls commented 1 year ago

Coverage Status

coverage: 88.639% (-1.1%) from 89.716% when pulling e8574e3b74b39cdfede08aacb24d2bf279e9d5a0 on Fish-Nullify:issues-and-commits-installation-id into 69430a8f014ebee576e196aea419391556f6036c on go-playground:master.

robinlieb commented 1 year ago

Hi @Fish-Nullify, Since this field is optional on the mentioned payloads, would it make sense to treat the installation struct accordingly? Something like:

Installation *struct {
    ID int64 `json:"id"`
} `json:"installation,omitempty"`

@deankarn do you have an opinion on that?

Fish-Nullify commented 1 year ago

Hi @robinlieb,

Thanks for the suggestion and I have provided an update to the installation structs for the specified payloads. I can easily revert this if necessary.

deankarn commented 1 year ago

@robinlieb I agree, if it’s optional then let’s make it a pointer.

@Fish-Nullify omitempty only works for Serialization in Go and not Deserialization.

robinlieb commented 1 year ago

@Fish-Nullify could you them please make the installation struct a pointer and remove the omitempty? Like:

Installation *struct {
    ID int64 `json:"id"`
} `json:"installation"`
Fish-Nullify commented 1 year ago

@robinlieb sorry for the late reply was away, but made the installation struct a pointer and removed the omitempty