-
Notifications
You must be signed in to change notification settings - Fork 483
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
Adds support for msg_id & thread_id #190
base: main
Are you sure you want to change the base?
Conversation
Can you pass in the new params directly instead? The current change may be a bit misleading, and we can pass in default params for |
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.
have msgId and threadId as parameters with default values
Having Wit.message() to receive more params it's not maintainable. It just increases the Cyclomatic complexity. A good approach to solve this problem is to have an object with the params we want. See issue #189. |
I would argue the complexity is higher if a single argument takes different forms. I'm not sure how adding two params with default values makes thing not maintainable. |
if we pass a single object we can just iterate over the properties of such object instead of adding more Regarding the maintainability, what will happen if suddenly the |
Updated the PR to show what the implementation look like |
Fixes issue #189, #191 & #169.