-
Notifications
You must be signed in to change notification settings - Fork 8
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 documentation on slack mentions #150
Conversation
documentation/config_docs.md
Outdated
@@ -41,6 +45,9 @@ Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks | |||
| `status_rules` | status rules config object | all status notifications are ignored | | |||
| `project_owners` | project owners config object | no project owners are defined | | |||
| `ignored_users` | list of users to be ignored on all notifications | no user is ignored | | |||
| `user_mappings` | list of mappings from git email and/or GitHub handle to Slack email | no mapping defined | |||
|
|||
Note that in `user_mappings`, git email to Slack email mappings are used for status DMs, while GitHub handle to Slack email mappings are used to get Slack mentions in notifications. |
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.
not explained why need to map by handle when email also available, the latter should be always better?
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.
Email is only available for commits and pushes, and it's the one from git config user.email
if I understand correctly, and that's what I meant by "git email" in the readme. So it's possible to match between emails for CI status DMs. In other types of comments we only have access to the GitHub handle. I can make this clearer in documentation.
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.
I think @rr0gi meant to explain the design considerations and decisions on the "GitHub handle to Slack email mappings are used to get Slack mentions in notifications" part, i.e. why not use git email to Slack email too?
@phongulus anything missing to merge this PR? |
Sorry, I completely forgot about this. I'll look again, add architectural comments as per @tysg's suggestion and re-request review. |
849c273
to
2ec8409
Compare
Sorry for the delay. @Khady for some reason I can't request a review from you. Can you take a look, and if it looks OK i think we can merge. |
Add some documentation on setting up Slack mentions and expected behaviour.