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 built-in middleware (directMention) document #756

Conversation

andrewdietrich
Copy link

Summary

Fixes #242. Updates docs with example of directMention and further details.

Requirements (place an x in each [ ])

@gitwave gitwave bot added the untriaged label Jan 14, 2021
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #756 (1a20b8f) into main (dfc61e7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #756   +/-   ##
=======================================
  Coverage   65.00%   65.00%           
=======================================
  Files          11       11           
  Lines        1086     1086           
  Branches      320      320           
=======================================
  Hits          706      706           
  Misses        321      321           
  Partials       59       59           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfc61e7...1a20b8f. Read the comment docs.

@misscoded misscoded added docs M-T: Documentation work only enhancement M-T: A feature request for new functionality and removed untriaged labels Jan 14, 2021
Copy link
Contributor

@shaydewael shaydewael left a comment

Choose a reason for hiding this comment

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

Hey @andrewcashmore 👋 thanks for the PR. I took a look, and definitely agree that there should be more clarity around how default listeners like directMention() work. However, I don't think we should make the custom listener explanation secondary since the Listener Middleware section is really intended to explain how developers can build custom listener middleware (similar to the framing of Global Middleware). The builtin middleware is quite limited and doesn't support the majority of developer uses of listener middleware (like validating that the user invoking the event has relevant permissions, or injecting additional information in the context object).

That said, we should surface the information you've included here, and keep the edit about the mention being at the beginning of the message because I think that's a helpful addition. But I think a better home for the rest may be in the reference documentation, which could explicitly list and explain a variety of built-in middleware (including directMention()).

@CLAassistant
Copy link

CLAassistant commented Feb 11, 2021

CLA assistant check
All committers have signed the CLA.

@seratch
Copy link
Member

seratch commented Mar 23, 2021

But I think a better home for the rest may be in the reference documentation, which could explicitly list and explain a variety of built-in middleware (including directMention()).

I agree with this.

@andrewdietrich If you are still interested in working on this and you're fine to move this (and other built-in middleware too if possible..), it'd be appreciated 🙇

@seratch seratch changed the title Issue 242 directmention documentation Add built-in middleware (directMention) document Mar 23, 2021
@seratch
Copy link
Member

seratch commented Aug 27, 2021

👋 To better manage tasks in this project, we are closing inactive issues and pull requests.

As we discussed above, we may add documents about built-in middleware as part of the Reference guide in the future. If you are still interested in making the document addition, we are happy to work together with you!

Thanks again for taking the time to make this pull request 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only enhancement M-T: A feature request for new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

directMention() helper function (documentation)
5 participants