-
Notifications
You must be signed in to change notification settings - Fork 400
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: show descriptions of function listener arguments #2364
docs: show descriptions of function listener arguments #2364
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2364 +/- ##
=======================================
Coverage 92.59% 92.59%
=======================================
Files 36 36
Lines 7472 7472
Branches 653 653
=======================================
Hits 6919 6919
Misses 545 545
Partials 8 8 ☔ View full report in Codecov by Sentry. |
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.
📝 Quick note on a confusion I had while making these changes!
docs/content/reference.md
Outdated
@@ -39,15 +39,15 @@ There are a collection of constraint objects that some methods have access to. T | |||
### Listener function arguments | |||
Listener functions have access to a set of arguments that may change based on the method which the function is passed to. Below is an explanation of the different arguments. The below table details the different arguments and the methods they'll be accessible in. | |||
|
|||
| Argument | Description | | |||
| :--- | :--- | | |||
| Argument | Method | Description | |
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.
🤔 The language is a bit confusing here for me... I had "listener" at first, but the other sections describe this column as a "method".
I believe "method" is right after some thinking? As I understand, "listeners" are actually "listener functions" and these are the functions that are called for certain methods. I'd love to learn more nuance on this though!
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.
A sentense like "payload
is available for all app.* listeners" makes sense to me, so I don't see issues with "Listener" here. Also, the last two rows actually have "All listeners" for the column. That said, "method" works too and it should be understood as well, so I don't have strong opinions on the naming, but personally I am down "Listener" here.
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.
Thanks! This is helpful and I'm glad to know both can make sense. I think that (and the use of "all listeners" below) was confusing me most 😉
I'll update this to "Listener" to avoid causing confusion within these tables!
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.
Either "listener" or "method" works for me! but left comments on the topic
docs/content/reference.md
Outdated
@@ -39,15 +39,15 @@ There are a collection of constraint objects that some methods have access to. T | |||
### Listener function arguments | |||
Listener functions have access to a set of arguments that may change based on the method which the function is passed to. Below is an explanation of the different arguments. The below table details the different arguments and the methods they'll be accessible in. | |||
|
|||
| Argument | Description | | |||
| :--- | :--- | | |||
| Argument | Method | Description | |
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.
A sentense like "payload
is available for all app.* listeners" makes sense to me, so I don't see issues with "Listener" here. Also, the last two rows actually have "All listeners" for the column. That said, "method" works too and it should be understood as well, so I don't have strong opinions on the naming, but personally I am down "Listener" here.
@@ -37,15 +37,15 @@ Slack アプリは通常、Slack からのイベント情報を受け取った | |||
### リスナー関数の引数 {#listener-function-arguments} | |||
リスナー関数がアクセスできる引数は、リスナー関数が渡されるメソッドによって決まります。以下の表は、これらの引数の説明です。この表は、それぞれの引数とそれにアクセスできるメソッドの詳細をカバーします。 | |||
|
|||
| 引数 | 説明 | | |||
| :--- | :--- | | |||
| 引数 | メソッド | 説明 | |
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.
if you go with "listener", it's "リスナー" in Japanese
@seratch Thank you for sharing reassurance in the method vs. listener mixup! And also for the review!! 🙏 ✨ Going to merge this now and see if I can watch that page change in real time 😳 |
if i wasn't already too late, i'd approve this! |
Summary
This PR adds another column to the "Listener function arguments" section of documentation to reveal a column hidden from the build! 📚 ✨
Preview
Before changes
After changes
Reviewers
Build these changes and inspect the following links:
Requirements