Skip to content

Confirm showNotifications setting optimization in useStreamStats#49

Merged
southernmagilla merged 1 commit intofix/rtmp-crashesfrom
copilot/sub-pr-48
Feb 1, 2026
Merged

Confirm showNotifications setting optimization in useStreamStats#49
southernmagilla merged 1 commit intofix/rtmp-crashesfrom
copilot/sub-pr-48

Conversation

Copy link
Contributor

Copilot AI commented Feb 1, 2026

Review comment requested verification that showNotifications setting is no longer read multiple times in handleAutoRetry.

Current Implementation

The setting is read once at function entry and reused throughout:

const handleAutoRetry = useCallback(async (groupId: string) => {
  // Read once at line 65
  const showNotifications = useSettingsStore.getState().showNotifications;
  
  // Reused at lines 72, 86, 99
  if (showNotifications) {
    showSystemNotification(/* ... */);
  }
}, [t]);

This eliminates the duplicate getState() calls that were present in the original implementation.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI mentioned this pull request Feb 1, 2026
Copilot AI changed the title [WIP] Address feedback on RTMP crash fix implementation Confirm showNotifications setting optimization in useStreamStats Feb 1, 2026
Copilot AI requested a review from southernmagilla February 1, 2026 00:04
@southernmagilla southernmagilla marked this pull request as ready for review February 1, 2026 00:09
@southernmagilla southernmagilla merged commit 8dd88bb into fix/rtmp-crashes Feb 1, 2026
1 check passed
@github-project-automation github-project-automation bot moved this from Todo to Done in SpiritStream Roadmap Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants