-
Notifications
You must be signed in to change notification settings - Fork 105
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
feat: implement InputMediaBuilder #210
Conversation
575b03b
to
ba0b4da
Compare
@KnorpelSenf Plz take a look the code, if it's good for you, then I will write the documentation for it. |
Thank you so much! I will be able to take a closer look on Wednesday, but perhaps someone else will have a look before me. |
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.
I recommend changing the file name to something simpler (one word), or to input_media_builder.ts
. So it is consistent with either the other grammY files or official Deno libraries.
I think we can rename it to I prefer the second one, it looks more accurate. |
The static method I don't know which one is better? |
2741b71
to
01a6a62
Compare
I have added them both, in case someone like the other style more. |
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.
What do you think about also providing something like InputMediaBuilder.Photo
and the like? That way, we could create stuff using InputMediaBuilder.Photo.from(...)
which can be passed to InputMediaBuilder.from
in turn. In addition, it allows for new InputMediaBuilder.Photo()
with chained method calls. The main builder could then also support the sub-builders, so that build
does not have to be called on all of them.
I am also wondering if there is a better way to create these things than specifying a config object for every entry. Perhaps this is already possible with the idea described above?
I have a idea about this. Add a
Then use a // This builder can only add `InputMediaPhoto` object.
const builder = InputMediaBuilder.fromType<InputMediaPhoto>(media, "photo"); Let me show you the code to explain it more. |
ae52764
to
1ddcdac
Compare
Please also add some tests. |
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.
I now think it is overkill to have a builder for an array. I remember thinking that I would like to abstract that away, but now I believe this would be overkill and take it too far.
I used the rest your changes to create #437. This will subsequently be closed. Thank you anywy for this contribution (and sorry for the long delay)!
Create
builders
subdirectory which other builders can put themselves in it.InputMediaBuilder methods:
Basic methods:
Advanced methods:
All these methods use
Omit
to removetype
property for each differentInputMedia
type, so that user can use it more convenient.Static method:
from
method to create a InputMediaBuilder withInputMedia
.Furthermore, it have
constructor
to initialize with someInputMedia
.It means if you like OOP, you can use
new InputMediaBuilder(/** data*/)
, or you can useInputMediaBuilder.from(/** data*/)
if you like FP more.TODO:
Close #101