You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@jzhang20133 Thank you opening this PR! I left a comment below, but David Brochart's review covers the areas of feedback blocking this PR from being merged. 😁
All I would like to add is that it's possible to emit events through a decorator interface. Emitting events imperatively (as done in this PR) is less preferable, since it's really easy for a developer to accidentally delete the line emitting the event. Using a decorator also allows for this feature to be added without any changes to the method definition, which I take as a sign of clean code.
To implement a decorator interface, you would want to define an @emit_awareness_event decorator that looks something like:
All I would like to add is that it's possible to emit events through a decorator interface. Emitting events imperatively (as done in this PR) is less preferable, since it's really easy for a developer to accidentally delete the line emitting the event. Using a decorator also allows for this feature to be added without any changes to the method definition, which I take as a sign of clean code.
To implement a decorator interface, you would want to define an
@emit_awareness_event
decorator that looks something like:After doing so, the method declarations would look like:
Let me know if this all sounds like a good idea for future work. If so, I can track this in a new issue so as to not block this PR. Thanks! 🤗
Originally posted by @dlqqq in #246 (review)
The text was updated successfully, but these errors were encountered: