-
Notifications
You must be signed in to change notification settings - Fork 75
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 typed options for createRoom #238
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally lgtm. It'd be nicer if this was autogenerated (which is why I didn't do it before), but that's not exactly feasible at the moment. Autogeneration would also have more value outside of the bot-sdk and instead be elevated to the spec level - talk to me before pursuing, if you plan to pursue.
Most of my comments are human linter style. Sorry the linter is so crap :(
Yeah, I figured it would probably be better if we kicked of an effort to autogenerate types from spec. However, this PR is here to scratch an itch atm rather than rewrite the world (sorry). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise lgtm
After the Nth time of checking the spec for the valid options of createRoom, I got bored and wrote types for it instead. This PR features a few opinionated things that I'd like to check for sanity, but I think this is a good improvement.
Signed off by: Will Hunt [email protected]