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

Clarify firing of import_notification_stream in doc comment #5811

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ffarall
Copy link
Contributor

@ffarall ffarall commented Sep 24, 2024

Description

Updates the doc comment on the import_notification_stream to make its behaviour clearer.

Closes Unexpected behaviour of block import_notification_stream.

Integration

Doesn't apply.

Review Notes

The old comment docs caused some confusion to myself and some members of my team, on when this notification stream is triggered. This is reflected in the linked issue, and as discussed there, this PR aims to prevent this confusion in future devs looking to make use of this functionality.

Checklist

  • My PR includes a detailed description as outlined in the "Description" and its two subsections above.
  • My PR follows the labeling requirements of this project (at minimum one label for T required)
    • External contributors: ask maintainers to put the right label on your PR.
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

You can remove the "Checklist" section once all have been checked. Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected behaviour of block import_notification_stream
1 participant