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

remove "debug" property #477

Closed
gr2m opened this issue Feb 18, 2021 · 6 comments · Fixed by #533
Closed

remove "debug" property #477

gr2m opened this issue Feb 18, 2021 · 6 comments · Fixed by #533
Labels
Type: Feature New feature or request
Milestone

Comments

@gr2m
Copy link
Contributor

gr2m commented Feb 18, 2021

We introduced a log option as a replacement for the debug module in #457. As part of the next breaking release, we shoudl remove the debug module altogether.

@gr2m gr2m added this to the v9 milestone Feb 18, 2021
@gr2m gr2m added the Type: Feature New feature or request label Mar 1, 2021
@gr2m
Copy link
Contributor Author

gr2m commented Mar 27, 2021

@G-Rath @oscard0m @wolfy1339 I'd like to remove the debug option in preparation for using webhooks as part of the octokit package, see octokit/octokit.js#15.

Any other breaking changes we should address as part of v9?

@wolfy1339
Copy link
Member

Possibly not related, but I think we should split up @octokit/webhooks-defintions and then include it in the changes for v9

@gr2m
Copy link
Contributor Author

gr2m commented Mar 27, 2021

That doesn't sound like a breaking change?

@G-Rath
Copy link
Member

G-Rath commented Mar 27, 2021

we could adjust the transform option to require it to return the event + extras, vs technically allowing anything to be returned (i.e strings).

I'm not fussed on how far we actually take this at runtime, since I think it'll be overkill to verify the returned object is most definitely the event by way of checking for every required property + types + etc but we could do a basic typeof x === 'object' assertion.

That's if we want to do anything at all at runtime - I'm happy if we just want to say "your transformation should return an extension of the original event object - our types assume you do but we don't actually enforce this at runtime" (which is what we're currently doing)

@gr2m
Copy link
Contributor Author

gr2m commented Mar 27, 2021

I agree, I wouldn't do any checks at runtime, just document that it should, and implement the types accordingly. Similarly, we don't implement any client-side validation for all the REST API endpoint methods any more, we just provide the correct types. It has worked out great

@gr2m gr2m mentioned this issue Mar 30, 2021
3 tasks
@gr2m gr2m mentioned this issue Apr 14, 2021
@gr2m gr2m closed this as completed in #533 Apr 14, 2021
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 9.0.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
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants