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

Add the 'PushEvent' webhook and associated PushCommit object #386

Merged
merged 4 commits into from Sep 10, 2023

Conversation

drewroengoogle
Copy link
Contributor

  • This adds some of the information for the PushEvent from Github
  • Additionally, created PushGitCommit that contains some data for the list of commits and head commit for PushEvent

Copy link
Collaborator

@ricardoamador ricardoamador left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ricardoamador
Copy link
Collaborator

cc @robbecker-wf is it possible to get this into a release?

@robrbecker
Copy link
Member

Looking now... sorry for the delay.

@robrbecker robrbecker added semver:minor Non-Breaking Public API Changes (additions) unreleased labels Sep 10, 2023
@robrbecker robrbecker merged commit a3b0006 into SpinlockLabs:master Sep 10, 2023
5 checks passed
@robbecker-wf
Copy link
Contributor

Released in version 9.18.0 https://github.com/SpinlockLabs/github.dart/releases/tag/9.18.0

@drewroengoogle
Copy link
Contributor Author

drewroengoogle commented Sep 14, 2023

Hi @robbecker-wf, I noticed there is a discrepancy in the Repository object defined in the PushEvent object here and the Repository object we are being sent in the PushEvent webhook, which will cause an error when converting the json to this new PushEvent object. Would it make more sense to revert this change, or instead open a new PR and fix it forward? Let me know your thoughts.

@robrbecker
Copy link
Member

Hmm good question. Would it be easier for Google if we just reverted? I don't see other issues filed with this problem, but that doesn't mean there aren't people out there affected.

@ricardoamador
Copy link
Collaborator

Can you revert the change but release the package under the same version as that with the change? That way no one has to update and technically have two versions with the same code?

@drewroengoogle
Copy link
Contributor Author

Can you revert the change but release the package under the same version as that with the change? That way no one has to update and technically have two versions with the same code?

This would probably be best if possible. I would also rather revert if we can.

@robrbecker
Copy link
Member

robrbecker commented Sep 14, 2023

Once a version is published to the pub server, it's there. So the revert will have to be 9.19.0

robrbecker added a commit that referenced this pull request Sep 14, 2023
…#387)

* Revert "Add the 'PushEvent' webhook and associated PushCommit object (#386)"

This reverts commit a3b0006.

* update changelog

---------

Co-authored-by: Rob Becker <rob.becker@workiva.com>
@robrbecker
Copy link
Member

Reverted in #387
published 9.19.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released semver:minor Non-Breaking Public API Changes (additions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants