Add more notifications docs about platform notifications and multiple channels #10801
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes enhance the DataHub's Subscriptions and Notifications feature by expanding notification support to include email along with Slack. Additionally, these improvements detail platform-level notifications and provide guidance on configuring multiple notification channels, offering users more flexibility and control over their notifications. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/managed-datahub/subscription-and-notification.md (3 hunks)
Additional context used
LanguageTool
docs/managed-datahub/subscription-and-notification.md
[uncategorized] ~8-~8: Possible missing comma found.
Context: ...Hub supports notifications on Slack and email with support for Microsoft Teams subscr...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~15-~15: Possible missing comma found.
Context: ...n the know about potential data quality issues so you can proactively manage your data...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~22-~22: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...he Platform section - this is different than "My Notifications"). The platform-leve...(DIFFERENT_THAN)
[grammar] ~25-~25: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...or removed from a data asset" then the slack channel or email you've set up in this ...(ON_SKYPE)
[style] ~42-~42: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...irst step is identifying the assets you want to subscribe to. DataHub’s [Lineage and I...(REP_WANT_TO_VB)
[grammar] ~179-~179: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...you can configure one default email and slack channel as well as overwrite that email...(ON_SKYPE)
[uncategorized] ~185-~185: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... the groups. 3. Go to each groups page (eg. through Settings > Users & Groups > Gro...(E_G)
[grammar] ~186-~186: In this context, ‘type’ should agree in number with the noun after ‘of’.
Context: ...4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email...(TYPE_OF_PLURAL)
[grammar] ~186-~186: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ... want, and add the appropriate email or slack channel id. 5. Now, when you visit an a...(ON_SKYPE)
Markdownlint
docs/managed-datahub/subscription-and-notification.md
20-20: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
21-21: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
24-24: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
42-42: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
175-175: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
180-180: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
183-183: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
17-17: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
124-124: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
136-136: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
142-142: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
143-143: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
111-111: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
183-183: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
11-11: null
Images should have alternate text (alt text)(MD045, no-alt-text)
51-51: null
Images should have alternate text (alt text)(MD045, no-alt-text)
57-57: null
Images should have alternate text (alt text)(MD045, no-alt-text)
64-64: null
Images should have alternate text (alt text)(MD045, no-alt-text)
72-72: null
Images should have alternate text (alt text)(MD045, no-alt-text)
78-78: null
Images should have alternate text (alt text)(MD045, no-alt-text)
85-85: null
Images should have alternate text (alt text)(MD045, no-alt-text)
96-96: null
Images should have alternate text (alt text)(MD045, no-alt-text)
102-102: null
Images should have alternate text (alt text)(MD045, no-alt-text)
108-108: null
Images should have alternate text (alt text)(MD045, no-alt-text)
152-152: null
Images should have alternate text (alt text)(MD045, no-alt-text)
163-163: null
Images should have alternate text (alt text)(MD045, no-alt-text)
| the correct permissions to do so) go to Settings > Notifications (under the Platform section - this is different than "My Notifications"). | ||
|
|
||
| The platform-level notifications are global notifications. This means if you tick the box for "An owner is added or removed from a data asset" | ||
| then the slack channel or email you've set up in this section will get a notification any time any asset has an owner added/removed. |
There was a problem hiding this comment.
Capitalize "Slack" when referring to the company or platform.
The word "slack" should be capitalized to refer correctly to the company or platform.
- then the slack channel or email you've set up in this section will get a notification
+ then the Slack channel or email you've set up in this section will get a notificationCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| then the slack channel or email you've set up in this section will get a notification any time any asset has an owner added/removed. | |
| then the Slack channel or email you've set up in this section will get a notification any time any asset has an owner added/removed. |
Tools
LanguageTool
[grammar] ~25-~25: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...or removed from a data asset" then the slack channel or email you've set up in this ...(ON_SKYPE)
There was a problem hiding this comment.
@jayacryl, good catch! Using "any time" as two words is indeed more appropriate in this formal context. Here's the updated suggestion:
- then the Slack channel or email you've set up in this section will get a notification any time any asset has an owner added/removed.Thank you for pointing this out!
|
|
||
| Datahub provides three levels of notifications: platform-level, group-level, and user-level. The group and user -level notifications are | ||
| described in the other sections of this doc. To set platform-level notifications (assuming you have | ||
| the correct permissions to do so) go to Settings > Notifications (under the Platform section - this is different than "My Notifications"). |
There was a problem hiding this comment.
Consider using "from" instead of "than" for standard English usage.
The phrase "different than" is less formal and can be replaced with "different from" to maintain a more standard style.
- this is different than "My Notifications"
+ this is different from "My Notifications"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| the correct permissions to do so) go to Settings > Notifications (under the Platform section - this is different than "My Notifications"). | |
| the correct permissions to do so) go to Settings > Notifications (under the Platform section - this is different from "My Notifications"). |
Tools
LanguageTool
[style] ~22-~22: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...he Platform section - this is different than "My Notifications"). The platform-leve...(DIFFERENT_THAN)
There was a problem hiding this comment.
might be worth bolding or giving special style to "Settings", "Notifications" and "My Notifications"
There was a problem hiding this comment.
@jayacryl, that's a great suggestion to enhance the clarity and visual guidance of the documentation. Here's how you can modify the markdown to apply bold styling to the terms "Settings", "Notifications", and "My Notifications":
Datahub provides three levels of notifications: platform-level, group-level, and user-level. The group and user-level notifications are described in the other sections of this doc. To set platform-level notifications (assuming you have the correct permissions to do so) go to **Settings** > **Notifications** (under the Platform section - this is different from **"My Notifications"**).This change will make these terms more prominent, helping users to quickly identify these important sections.
| I want to configure multiple channels. How many slack channels or emails can I configure to get notified? | ||
| </summary> | ||
| At the platform-level, you can configure one email and one slack channel. | ||
|
|
||
| At the user and group -levels, you can configure one default email and slack channel as well as overwrite that email/channel when you | ||
| go to a specific asset to subscribe to. | ||
|
|
||
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | ||
| 1. Create a datahub group for each channel you want notifications for. | ||
| 2. Add yourself as a member to each of the groups. | ||
| 3. Go to each groups page (eg. through Settings > Users & Groups > Group Name OR by searching for your group's name). | ||
| 4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email or slack channel id. | ||
| 5. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". | ||
|
|
There was a problem hiding this comment.
Clarify multiple channel configuration steps and fix minor issues.
Several grammatical and stylistic improvements can be made to enhance clarity and professionalism.
- At the platform-level, you can configure one email and one slack channel.
+ At the platform level, you can configure one email and one Slack channel.
- At the user and group -levels, you can configure one default email and slack channel as well as overwrite that email/channel when you
+ At the user and group levels, you can configure one default email and Slack channel, as well as overwrite that email/channel when you
- 3. Go to each groups page (eg. through Settings > Users & Groups > Group Name OR by searching for your group's name).
+ 3. Go to each group's page (e.g., through Settings > Users & Groups > Group Name or by searching for your group's name).
- 4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email or slack channel id.
+ 4. Click on 'Notifications', enable the types of notifications you want, and add the appropriate email or Slack channel ID.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| I want to configure multiple channels. How many slack channels or emails can I configure to get notified? | |
| </summary> | |
| At the platform-level, you can configure one email and one slack channel. | |
| At the user and group -levels, you can configure one default email and slack channel as well as overwrite that email/channel when you | |
| go to a specific asset to subscribe to. | |
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | |
| 1. Create a datahub group for each channel you want notifications for. | |
| 2. Add yourself as a member to each of the groups. | |
| 3. Go to each groups page (eg. through Settings > Users & Groups > Group Name OR by searching for your group's name). | |
| 4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email or slack channel id. | |
| 5. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". | |
| I want to configure multiple channels. How many slack channels or emails can I configure to get notified? | |
| </summary> | |
| At the platform level, you can configure one email and one Slack channel. | |
| At the user and group levels, you can configure one default email and Slack channel, as well as overwrite that email/channel when you | |
| go to a specific asset to subscribe to. | |
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | |
| 1. Create a datahub group for each channel you want notifications for. | |
| 2. Add yourself as a member to each of the groups. | |
| 3. Go to each group's page (e.g., through Settings > Users & Groups > Group Name or by searching for your group's name). | |
| 4. Click on 'Notifications', enable the types of notifications you want, and add the appropriate email or Slack channel ID. | |
| 5. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". |
Tools
LanguageTool
[grammar] ~179-~179: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...you can configure one default email and slack channel as well as overwrite that email...(ON_SKYPE)
[uncategorized] ~185-~185: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... the groups. 3. Go to each groups page (eg. through Settings > Users & Groups > Gro...(E_G)
[grammar] ~186-~186: In this context, ‘type’ should agree in number with the noun after ‘of’.
Context: ...4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email...(TYPE_OF_PLURAL)
[grammar] ~186-~186: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ... want, and add the appropriate email or slack channel id. 5. Now, when you visit an a...(ON_SKYPE)
Markdownlint
175-175: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
180-180: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
183-183: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
183-183: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
| You'll also have the option to turn on notifications for ingestion failures (very useful to route to a platform channel to proactively ensure your | ||
| Datahub metadata stays fresh). |
There was a problem hiding this comment.
Though things like tag changes may create a lot of noisy, we recommend turning on notifications for ingestion failures. This will help you proactively ensure...
Or something like:
*Our recommendations:*
Ingestion failures: Turning global notifications on for these will...
| 3. Go to each groups page (eg. through Settings > Users & Groups > Group Name OR by searching for your group's name). | ||
| 4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email or slack channel id. |
There was a problem hiding this comment.
if you want for more simplicity: these two steps can be skipped now because when they subscribe to an asset we automatically turn on notifications and set default channel/email
|
|
||
| ## Platform Admin Notifications | ||
|
|
||
| Datahub provides three levels of notifications: platform-level, group-level, and user-level. The group and user -level notifications are |
There was a problem hiding this comment.
Because this page is text-heavy, might want to break this out into a bulleted list. Visual structure might help catch people's attention and make it easier to digest:
| ## Platform Admin Notifications | ||
|
|
||
| Datahub provides three levels of notifications: platform-level, group-level, and user-level. The group and user -level notifications are | ||
| described in the other sections of this doc. To set platform-level notifications (assuming you have |
There was a problem hiding this comment.
I would break 'To set' into a newline or maybe subtitle font style kind of thing - just bc it's action oriented and different from the earlier text that just gives initial context. Makes it easy to miss otherwise if someone's skimming
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/managed-datahub/subscription-and-notification.md (2 hunks)
Additional context used
LanguageTool
docs/managed-datahub/subscription-and-notification.md
[uncategorized] ~8-~8: Possible missing comma found.
Context: ...Hub supports notifications on Slack and email with support for Microsoft Teams subscr...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~15-~15: Possible missing comma found.
Context: ...n the know about potential data quality issues so you can proactively manage your data...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~36-~36: The verb ‘recommend’ is used with the gerund form.
Context: ... the platform would be noisy, and so we recommend to use these platform-level notifications only...(ADMIT_ENJOY_VB)
[style] ~50-~50: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...irst step is identifying the assets you want to subscribe to. DataHub’s [Lineage and I...(REP_WANT_TO_VB)
[uncategorized] ~120-~120: Possible missing comma found.
Context: ... the earlier sections. However, in some cases you may want to only be notified about ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~187-~187: Possible missing comma found.
Context: ...n configure one default email and Slack channel as well as overwrite that email/channel...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
docs/managed-datahub/subscription-and-notification.md
27-27: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
28-28: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
31-31: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
32-32: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
50-50: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
183-183: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
188-188: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
191-191: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
17-17: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
132-132: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
144-144: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
150-150: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
151-151: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
119-119: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
27-27: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
31-31: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
191-191: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
34-34: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
11-11: null
Images should have alternate text (alt text)(MD045, no-alt-text)
59-59: null
Images should have alternate text (alt text)(MD045, no-alt-text)
65-65: null
Images should have alternate text (alt text)(MD045, no-alt-text)
72-72: null
Images should have alternate text (alt text)(MD045, no-alt-text)
80-80: null
Images should have alternate text (alt text)(MD045, no-alt-text)
86-86: null
Images should have alternate text (alt text)(MD045, no-alt-text)
93-93: null
Images should have alternate text (alt text)(MD045, no-alt-text)
104-104: null
Images should have alternate text (alt text)(MD045, no-alt-text)
110-110: null
Images should have alternate text (alt text)(MD045, no-alt-text)
116-116: null
Images should have alternate text (alt text)(MD045, no-alt-text)
160-160: null
Images should have alternate text (alt text)(MD045, no-alt-text)
171-171: null
Images should have alternate text (alt text)(MD045, no-alt-text)
| @@ -5,18 +5,39 @@ | |||
| <FeatureAvailability saasOnly /> | |||
|
|
|||
| DataHub's Subscriptions and Notifications feature gives you real-time change alerts on data assets of your choice. | |||
| With this feature, you can set up subscriptions to specific changes for an Entity – and DataHub will notify you when those changes happen. Currently, DataHub supports notifications on Slack, with support for Microsoft Teams and email subscriptions forthcoming. | |||
| With this feature, you can set up subscriptions to specific changes for an Entity – and DataHub will notify you when those changes happen. Currently, DataHub supports notifications on Slack and email with support for Microsoft Teams subscriptions forthcoming. | |||
There was a problem hiding this comment.
Consider adding a comma for improved readability.
The sentence structure indicates a list of supported platforms, and a comma would aid in clearly separating the items.
- Slack and email with support for Microsoft Teams subscriptions forthcoming
+ Slack, and email with support for Microsoft Teams subscriptions forthcomingCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| With this feature, you can set up subscriptions to specific changes for an Entity – and DataHub will notify you when those changes happen. Currently, DataHub supports notifications on Slack and email with support for Microsoft Teams subscriptions forthcoming. | |
| With this feature, you can set up subscriptions to specific changes for an Entity – and DataHub will notify you when those changes happen. Currently, DataHub supports notifications on Slack, and email with support for Microsoft Teams subscriptions forthcoming. |
Tools
LanguageTool
[uncategorized] ~8-~8: Possible missing comma found.
Context: ...Hub supports notifications on Slack and email with support for Microsoft Teams subscr...(AI_HYDRA_LEO_MISSING_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/managed-datahub/subscription-and-notification.md (2 hunks)
Additional context used
LanguageTool
docs/managed-datahub/subscription-and-notification.md
[uncategorized] ~15-~15: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...assertions, or incidents. You’ll always been in the know about potential data qualit...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~15-~15: Possible missing comma found.
Context: ...n the know about potential data quality issues so you can proactively manage your data...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~36-~36: The verb ‘recommend’ is used with the gerund form.
Context: ... the platform would be noisy, and so we recommend to use these platform-level notifications only...(ADMIT_ENJOY_VB)
[style] ~50-~50: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...irst step is identifying the assets you want to subscribe to. DataHub’s [Lineage and I...(REP_WANT_TO_VB)
Markdownlint
docs/managed-datahub/subscription-and-notification.md
27-27: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
28-28: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
31-31: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
32-32: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
50-50: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
183-183: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
188-188: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
191-191: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
17-17: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
132-132: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
144-144: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
150-150: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
151-151: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
119-119: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
27-27: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
31-31: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
191-191: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
34-34: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
11-11: null
Images should have alternate text (alt text)(MD045, no-alt-text)
59-59: null
Images should have alternate text (alt text)(MD045, no-alt-text)
65-65: null
Images should have alternate text (alt text)(MD045, no-alt-text)
72-72: null
Images should have alternate text (alt text)(MD045, no-alt-text)
80-80: null
Images should have alternate text (alt text)(MD045, no-alt-text)
86-86: null
Images should have alternate text (alt text)(MD045, no-alt-text)
93-93: null
Images should have alternate text (alt text)(MD045, no-alt-text)
104-104: null
Images should have alternate text (alt text)(MD045, no-alt-text)
110-110: null
Images should have alternate text (alt text)(MD045, no-alt-text)
116-116: null
Images should have alternate text (alt text)(MD045, no-alt-text)
160-160: null
Images should have alternate text (alt text)(MD045, no-alt-text)
171-171: null
Images should have alternate text (alt text)(MD045, no-alt-text)
Additional comments not posted (1)
docs/managed-datahub/subscription-and-notification.md (1)
8-8: LGTM!The introduction clearly outlines the current and forthcoming notification capabilities. The mention of Microsoft Teams is particularly noteworthy as it indicates upcoming enhancements.
|
|
||
| <p align="center"> | ||
| <img width="70%" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-slack.png"/> | ||
| <img width="70%" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-Slack.png"/> |
There was a problem hiding this comment.
Add alt text to images for accessibility.
Images in the document are missing alternative text, which is important for accessibility and SEO.
- <img width="70%" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-Slack.png"/>
+ <img width="70%" alt="User notifications in Slack" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-Slack.png"/>Please apply similar changes to all other images in the document.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img width="70%" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-Slack.png"/> | |
| <img width="70%" alt="User notifications in Slack" src="https://raw.githubusercontent.com/datahub-project/static-assets/main/imgs/saas/subscription-and-notification/s_n-user-notifications-in-Slack.png"/> |
Tools
Markdownlint
11-11: null
Images should have alternate text (alt text)(MD045, no-alt-text)
| **Setting Platform-Level Notifications:** | ||
| - Note: this requires appropriate permissions. | ||
| - Go to `Settings` > `Notifications` (under the `Platform` section, not `My Notifications`). | ||
|
|
||
| **Global Notifications:** | ||
| - Platform-level notifications are applied to all assets within Datahub. | ||
| - Example: If "An owner is added or removed from a data asset" is ticked, the designated Slack channel or email will receive notifications for any such changes across all assets. | ||
|
|
||
| **Our Recommendations** |
There was a problem hiding this comment.
Improve document structure by using headings.
The use of bold text for sections that are significant enough to be headings could lead to poor document structure and readability.
- **Setting Platform-Level Notifications:**
+ ### Setting Platform-Level Notifications
- **Global Notifications:**
+ ### Global Notifications
- **Our Recommendations**
+ ### Our RecommendationsAdditionally, adjust the list indentation to comply with Markdown standards.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Setting Platform-Level Notifications:** | |
| - Note: this requires appropriate permissions. | |
| - Go to `Settings` > `Notifications` (under the `Platform` section, not `My Notifications`). | |
| **Global Notifications:** | |
| - Platform-level notifications are applied to all assets within Datahub. | |
| - Example: If "An owner is added or removed from a data asset" is ticked, the designated Slack channel or email will receive notifications for any such changes across all assets. | |
| **Our Recommendations** | |
| ### Setting Platform-Level Notifications | |
| - Note: this requires appropriate permissions. | |
| - Go to `Settings` > `Notifications` (under the `Platform` section, not `My Notifications`). | |
| ### Global Notifications | |
| - Platform-level notifications are applied to all assets within Datahub. | |
| - Example: If "An owner is added or removed from a data asset" is ticked, the designated Slack channel or email will receive notifications for any such changes across all assets. | |
| ### Our Recommendations |
Tools
Markdownlint
27-27: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
28-28: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
31-31: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
32-32: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
27-27: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
31-31: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
34-34: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
| I want to configure multiple channels. How many Slack channels or emails can I configure to get notified? | ||
| </summary> | ||
| At the platform-level, you can configure one email and one Slack channel. | ||
|
|
||
| At the user and group -levels, you can configure one default email and Slack channel as well as overwrite that email/channel when you | ||
| go to a specific asset to subscribe to. | ||
|
|
||
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | ||
| 1. Create a datahub group for each channel you want notifications for. | ||
| 2. Add yourself as a member to each of the groups. | ||
| 3. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". | ||
|
|
There was a problem hiding this comment.
Clarify multiple channel configuration steps and fix minor issues.
Several grammatical and stylistic improvements can be made to enhance clarity and professionalism.
- At the platform-level, you can configure one email and one Slack channel.
+ At the platform level, you can configure one email and one Slack channel.
- At the user and group -levels, you can configure one default email and Slack channel as well as overwrite that email/channel when you
+ At the user and group levels, you can configure one default email and Slack channel, as well as overwrite that email/channel when you
- 3. Go to each groups page (eg. through Settings > Users & Groups > Group Name OR by searching for your group's name).
+ 3. Go to each group's page (e.g., through Settings > Users & Groups > Group Name or by searching for your group's name).
- 4. Click on 'Notifications', enable the type of notifications you want, and add the appropriate email or slack channel id.
+ 4. Click on 'Notifications', enable the types of notifications you want, and add the appropriate email or Slack channel ID.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| I want to configure multiple channels. How many Slack channels or emails can I configure to get notified? | |
| </summary> | |
| At the platform-level, you can configure one email and one Slack channel. | |
| At the user and group -levels, you can configure one default email and Slack channel as well as overwrite that email/channel when you | |
| go to a specific asset to subscribe to. | |
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | |
| 1. Create a datahub group for each channel you want notifications for. | |
| 2. Add yourself as a member to each of the groups. | |
| 3. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". | |
| I want to configure multiple channels. How many Slack channels or emails can I configure to get notified? | |
| </summary> | |
| At the platform level, you can configure one email and one Slack channel. | |
| At the user and group levels, you can configure one default email and Slack channel, as well as overwrite that email/channel when you | |
| go to a specific asset to subscribe to. | |
| To configure multiple channels, as a prereq, ensure you have the appropriate privileges. And then: | |
| 1. Create a datahub group for each channel you want notifications for. | |
| 2. Add yourself as a member to each of the groups. | |
| 3. Now, when you visit an asset and go to subscribe, you'll see the option "Manage Group Subscriptions". |
Tools
Markdownlint
183-183: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
188-188: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
191-191: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
191-191: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
| - Note: this requires appropriate permissions. | ||
| - Go to `Settings` > `Notifications` (under the `Platform` section, not `My Notifications`). |
There was a problem hiding this comment.
Super nit but I think this specific section was actually more readable without bullets.
| - Platform-level notifications are applied to all assets within Datahub. | ||
| - Example: If "An owner is added or removed from a data asset" is ticked, the designated Slack channel or email will receive notifications for any such changes across all assets. |
There was a problem hiding this comment.
Samsies here. Might be subjective but I liked it without bullets but each item on its own line
| - Platform-level notifications are applied to all assets within Datahub. | ||
| - Example: If "An owner is added or removed from a data asset" is ticked, the designated Slack channel or email will receive notifications for any such changes across all assets. | ||
|
|
||
| **Our Recommendations** |
There was a problem hiding this comment.
Teeny nit, but adding colon at the end here for consistency may be worth.
| - Note: this requires appropriate permissions. | ||
| - Go to `Settings` > `Notifications` (under the `Platform` section, not `My Notifications`). | ||
|
|
||
| **Global Notifications:** |
There was a problem hiding this comment.
Should this not be called Platform-Level?
Agree with jay's comment above
There was a problem hiding this comment.
Ah good point - the term change threw me off when reading that bit at first too
datahub-cyborg
bot
added
the
needs-review
…els (datahub-project#10801) Co-authored-by: Jay <159848059+jayacryl@users.noreply.github.com>
…els (datahub-project#10801) Co-authored-by: Jay <159848059+jayacryl@users.noreply.github.com>
…els (datahub-project#10801) Co-authored-by: Jay <159848059+jayacryl@users.noreply.github.com>
Checklist
Summary by CodeRabbit