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

Update managing-scheduled-reminders-for-your-team.md with "Get mentioned on slack" section #31801

Merged
merged 8 commits into from
May 31, 2024

Conversation

ranyitz
Copy link
Contributor

@ranyitz ranyitz commented Feb 23, 2024

Why:

I've had to contact support to figure this out, so I thought it would be good to add it to the official docs.

Check off the following:

  • I have reviewed my changes in staging, available via the View deployment link in this PR's timeline (this link will be available after opening the PR).

    • For content changes, you will also see an automatically generated comment with links directly to pages you've modified. The comment won't appear if your PR only edits files in the data directory.
  • For content changes, I have completed the self-review checklist.

Copy link

welcome bot commented Feb 23, 2024

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Feb 23, 2024
Copy link
Contributor

github-actions bot commented Feb 23, 2024

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
organizations/managing-organization-settings/managing-scheduled-reminders-for-your-organization.md fpt
ghec
fpt
ghec
organizations/organizing-members-into-teams/managing-scheduled-reminders-for-your-team.md fpt
ghec
fpt
ghec

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server

@ranyitz
Copy link
Contributor Author

ranyitz commented Feb 23, 2024

Before i'm going to invest any time fixing the lint errors, please let me know if you'd like to accept this PR first. And ideally, what should be done from your perspective

@nguyenalex836 nguyenalex836 added content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review organizations Content related to organizations and removed triage Do not begin working on this issue until triaged by the team labels Feb 23, 2024
@nguyenalex836
Copy link
Contributor

@ranyitz Thanks so much for opening a PR! I'll get this triaged for review ✨

@felicitymay
Copy link
Contributor

Hi @ranyitz 👋🏻

Apologies for the delay in reviewing. Thanks for flagging your difficulty in finding the information you needed to make Slack reminders work properly.

I'd like to be sure that I understand your use case properly before reviewing this PR because it's possible that we should add information about reminders in private channels to more articles than just this one.

It sounds as if you were using private Slack channels for reminders and needed to go through some additional steps before the reminders worked. Something like the steps described in the README for the integration. Have I understood correctly, or was there more to the problem?

@ranyitz
Copy link
Contributor Author

ranyitz commented May 30, 2024

@felicitymay thanks for returning to me on that.

Yes we used private slack channels for reminders, i'll provide some additional context.
The reminders integration is less effective if the reminded user is not notified. In our case, some users were mentioned and thus notified, and other users were not. I couldn't find what needs to be done and approached the support:

image

Consider that the entry point for this feature is https://docs.github.com/en/organizations/organizing-members-into-teams/managing-scheduled-reminders-for-your-team and not https://github.com/integrations/slack?tab=readme-ov-file#getting-started

@felicitymay
Copy link
Contributor

Thanks for confirming your use case ✨

I'm going to look into this a little further because it seems as there are several articles in GitHub docs where this information is relevant. It might make sense to create a reusable and then include that text in the relevant articles. 🤔

I'll get back to you on this tomorrow.

Copy link

@Faphouse Faphouse left a comment

Choose a reason for hiding this comment

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

@felicitymay
Copy link
Contributor

@ranyitz - it seems as if this information is needed in two files:

The related article for users, Managing your scheduled reminders, doesn't seem to include the option to select a Slack channel.

I made some changes locally and tested them before committing them to your branch. I've expanded the information about choosing a Slack channel to let people know that if they choose a private channel, they'll need to ask users to run extra steps. In addition, I've added links to the README file for the integration.

It would be good to know if this would have answered your questions.

Copy link
Contributor Author

@ranyitz ranyitz left a comment

Choose a reason for hiding this comment

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

@felicitymay I added a review comment

@@ -1 +1,3 @@
1. Under "Slack channel", type the name of the Slack channel where you'd like to receive notifications.
> [!TIP]
> If this Slack channel is private, you will need to invite the integration into the channel: `/invite @github`. Alternatively, you can ask users to run `/github signin` in the Slack channel. For more information, see [Getting started](https://github.com/integrations/slack?tab=readme-ov-file#getting-started) in the Slack integrations documentation.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me correct it (to the best of my knowledge)

  1. Using /invite @github to a private channel is mandatory for the integration to work.
  2. Running /github signin is not an alternative to running /invite @github, in fact the integration works and send reminders without it. Running /github signin for each users will allow them to get tagged as part of the reminder.

The last part about tagging of users is the missing part that need to be somewhere in the docs. Please see my original version of the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When saying tagging I meant, mention. So the user gets a notification and as a result, aware that they need to review a PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your clear review comments. I've updated the tip.

@@ -1 +1,3 @@
1. Under "Slack channel", type the name of the Slack channel where you'd like to receive notifications.
> [!TIP]
> If this Slack channel is private, you will need to invite the integration into the channel: `/invite @github`. In addition, you need to ask users to run `/github signin` in the Slack channel otherwise they will not be @mentioned. For more information, see [Getting started](https://github.com/integrations/slack?tab=readme-ov-file#getting-started) in the Slack integrations documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your clear feedback. It sounds as if the README file for the integration might be slightly outdated 🤔

Is this now a correct description? Or do users always need to run /github signin if they want to be mentioned?

@felicitymay
Copy link
Contributor

I'm not sure why the preview keeps failing. I'm going to update the branch from main and see if that fixes the problem.

@felicitymay
Copy link
Contributor

Copy link
Contributor Author

@ranyitz ranyitz left a comment

Choose a reason for hiding this comment

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

lgtm ✅

Co-authored-by: Ran Yitzhaki <ranyitz@users.noreply.github.com>
@ranyitz
Copy link
Contributor Author

ranyitz commented May 31, 2024

@felicitymay I'm not able to merge this PR, so i'll leave it to you from here.

@felicitymay
Copy link
Contributor

I'm not able to merge this PR, so i'll leave it to you from here.

@ranyitz - many thanks for your collaboration on this update to the docs, and for flagging it in the first place. Once the tests have passed, I'll merge this and you'll see the changes in the live docs within a day.

@felicitymay felicitymay added this pull request to the merge queue May 31, 2024
Merged via the queue into github:main with commit f37c64c May 31, 2024
43 checks passed
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues

@ranyitz ranyitz deleted the patch-1 branch June 3, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team organizations Content related to organizations waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants