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

Do we really need clearAttachments method? #8

Closed
antonkomarev opened this issue Feb 22, 2020 · 3 comments · Fixed by #10
Closed

Do we really need clearAttachments method? #8

antonkomarev opened this issue Feb 22, 2020 · 3 comments · Fixed by #10
Labels
question Further information is requested

Comments

@antonkomarev
Copy link
Collaborator

antonkomarev commented Feb 22, 2020

@Funfare you've added clearAttachments method to RocketChatMessage class.
What is the use case? Do we really need it?

@antonkomarev antonkomarev added the question Further information is requested label Feb 22, 2020
@Funfare
Copy link
Collaborator

Funfare commented Feb 24, 2020

Without that there is no option to change/reset the attachments.
Don't know of we really ned that, but for me it is bad design when you can't change an attribute later. All other methods could be called several times

@antonkomarev
Copy link
Collaborator Author

I don't think it's bad design. As for me it looks really wrong when you are attaching files to message and then decide to remove all of them. That means you've missed some kind of conditional before attachments add process.

@Funfare
Copy link
Collaborator

Funfare commented Feb 24, 2020

ok. Then I think we can remove that method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
2 participants