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

docs: add JSDoc to and list out all available builtin middleware functions in the docs #2136

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Jun 14, 2024

Just adding some JSdocs to the available builtin middleware docs. Also added a list of them to the actual docs site.

This fixes #242 .

@filmaj filmaj added docs M-T: Documentation work only semver:patch labels Jun 14, 2024
@filmaj filmaj requested a review from a team June 14, 2024 22:12
@filmaj filmaj self-assigned this Jun 14, 2024
Copy link

codecov bot commented Jun 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.03%. Comparing base (0f9326e) to head (41609c7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2136   +/-   ##
=======================================
  Coverage   82.03%   82.03%           
=======================================
  Files          18       18           
  Lines        1536     1536           
  Branches      442      442           
=======================================
  Hits         1260     1260           
  Misses        178      178           
  Partials       98       98           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thanks for the work! I think the document could be clearer about which middleware is typically used for listener middleware. Technically, you can pass any middleware to any listener, but some patterns are just meaningless.

docs/_tutorials/reference.md Outdated Show resolved Hide resolved
@@ -288,6 +288,11 @@ export function matchEventType(pattern: EventTypePattern): Middleware<SlackEvent
};
}

// TODO: breaking change: why does this method have to be invoked as a function with no args, while other similar
// method like the `only*` ones do not require that? should make this consistent.
Copy link
Member

Choose a reason for hiding this comment

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

If we rename this to onlyNonSelfEvents or something like that, that sounds good to me too. When we make such a change, perhaps having an alias "ignoreSelf" for backward compatibility for a while would be a good way to go.

@filmaj filmaj requested a review from seratch June 17, 2024 18:09
@filmaj
Copy link
Contributor Author

filmaj commented Jun 17, 2024

@seratch I've split the built-in middleware list into global vs. listener lists; let me know what you think 🙇

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Looks goo to me 👍

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 semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

directMention() helper function (documentation)
3 participants