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 'create' github webhook event to hooks.dart #304

Merged
merged 5 commits into from Mar 29, 2022

Conversation

XilaiZhang
Copy link
Contributor

The create webhook event and its payloads are described here at : https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#create

The tests seem to fail pretty significantly but I looked around and they seem unrelated to this change, but are failing because of existing codes?

@XilaiZhang
Copy link
Contributor Author

seems like I can't add reviewers. cc @CaseyHillers and @keyonghan

@CaseyHillers CaseyHillers self-requested a review March 28, 2022 19:21
@CaseyHillers CaseyHillers added the semver:minor Non-Breaking Public API Changes (additions) label Mar 28, 2022
Copy link
Collaborator

@CaseyHillers CaseyHillers left a comment

Choose a reason for hiding this comment

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

This requires an update to the changelog and increase the Y version

User? sender;

Map<String, dynamic> toJson() => _$CreateEventToJson(this);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, added

@CaseyHillers
Copy link
Collaborator

You'll need to update CHANGELOG and the pubspec.yaml for the y version

@robrbecker
Copy link
Member

@CaseyHillers Actually, I've set up auto-release on PR merge, so updating the changelog and version in the pubspec happens automatically after merge based on the semver label. So the PR itself should not update those files.

@CaseyHillers
Copy link
Collaborator

@CaseyHillers Actually, I've set up auto-release on PR merge, so updating the changelog and version in the pubspec happens automatically after merge based on the semver label. So the PR itself should not update those files.

That is fancier than I imagined since we last talked about it :-) Nice work!

@XilaiZhang feel free to disregard my comment. However, there's some formatting issues

@robrbecker
Copy link
Member

@XilaiZhang it does look like the test file you added needs to be formatted. Use dart format --fix .

@XilaiZhang
Copy link
Contributor Author

Thanks for the instructions! (Also thanks for providing the format commands, previously I used the same format command for flutter/cocoon and formatted like 132 files :) and reverted

@CaseyHillers CaseyHillers merged commit 1a05e86 into SpinlockLabs:master Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor Non-Breaking Public API Changes (additions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants