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 typed options for createRoom #238

Merged
merged 9 commits into from
Jul 6, 2022

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Jul 1, 2022

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]

src/models/Room.ts Outdated Show resolved Hide resolved
src/models/Room.ts Outdated Show resolved Hide resolved
src/MatrixClient.ts Show resolved Hide resolved
Copy link
Owner

@turt2live turt2live left a 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 :(

examples/encryption_bot.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/models/Room.ts Outdated Show resolved Hide resolved
src/models/Room.ts Outdated Show resolved Hide resolved
src/models/Room.ts Outdated Show resolved Hide resolved
src/models/Room.ts Outdated Show resolved Hide resolved
@Half-Shot
Copy link
Contributor Author

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.

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).

@Half-Shot Half-Shot requested a review from turt2live July 5, 2022 20:27
Copy link
Owner

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

otherwise lgtm

src/MatrixClient.ts Outdated Show resolved Hide resolved
src/models/CreateRoom.ts Show resolved Hide resolved
src/models/CreateRoom.ts Show resolved Hide resolved
src/models/CreateRoom.ts Outdated Show resolved Hide resolved
src/models/CreateRoom.ts Show resolved Hide resolved
examples/encryption_bot.ts Outdated Show resolved Hide resolved
@turt2live turt2live enabled auto-merge (squash) July 6, 2022 05:05
@turt2live turt2live merged commit cd8d1f4 into turt2live:master Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants