-
Notifications
You must be signed in to change notification settings - Fork 0
Development #36
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
Development #36
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…4b50-93c4-0e11b13594f4 Clarify error handler ordering is correct - no changes needed
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…47b6-b204-5bf8892748d3 Extract magic numbers to named constants in audio playback code
…4606-8149-97ceaa22873d Use platform.system() instead of sys.platform for OS detection
Extract duplicated hex opacity calculation to utility function
Extract avatar active offset magic number into configurable setting
Fix audio reference cleanup on play() failure in popup mode
Fix race condition in popup avatar lifecycle
…into development
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.
Pull request overview
This PR appears to be a development branch merge that includes two main changes: (1) modifying the Twitch bot's channel joining behavior to rely on TwitchIO's automatic channel joining instead of explicit join_channels() calls, and (2) standardizing CI/CD workflows to use Python 3.10 exclusively.
Key Changes:
- Removed explicit
join_channels()call inevent_ready(), relying instead on TwitchIO 3.x's automatic joining behavior frominitial_channels - Added debug logging to verify connected channels and available bot methods
- Reduced Python version testing matrix from [3.9, 3.10, 3.11] to only [3.10] in test workflows
- Downgraded build/release workflows from Python 3.11 to Python 3.10
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| backend/modules/twitch_listener.py | Modified event_ready() to remove explicit channel joining and add debug logging for connected channels and bot methods |
| .github/workflows/tests.yml | Reduced Python test matrix from multiple versions (3.9, 3.10, 3.11) to only Python 3.10 |
| .github/workflows/build-and-release.yml | Downgraded Python version from 3.11 to 3.10 in both build and release jobs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| logger.info(f"Connected channels: {list(self.connected_channels)}") | ||
|
|
||
| # Log available methods for debugging | ||
| logger.debug(f"Bot methods: {[m for m in dir(self) if 'channel' in m.lower() or 'join' in m.lower()]}") |
Copilot
AI
Nov 25, 2025
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.
This debug logging statement appears to be temporary debugging code and should be removed before merging. Logging all methods with 'channel' or 'join' in their names could expose unnecessary internal implementation details and adds no value in production. Consider removing this line or moving it to a more targeted test/debugging context.
| logger.debug(f"Bot methods: {[m for m in dir(self) if 'channel' in m.lower() or 'join' in m.lower()]}") |
| strategy: | ||
| matrix: | ||
| python-version: ['3.9', '3.10', '3.11'] | ||
| python-version: ['3.10'] |
Copilot
AI
Nov 25, 2025
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.
Removing Python 3.9 and 3.11 from the test matrix reduces test coverage across supported Python versions. The README.md states the project requires "Python 3.9+" (line 130), but this change only tests Python 3.10. Consider either:
- Keeping the original matrix to test across all supported versions (3.9, 3.10, 3.11), or
- Updating the README.md to reflect the actual supported Python version if only 3.10 is supported.
This ensures consistency between documentation and CI testing.
| - name: Set up Python 3.10 | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.11' | ||
| python-version: '3.10' |
Copilot
AI
Nov 25, 2025
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.
This change downgrades the Python version from 3.11 to 3.10, which is inconsistent with the project's stated requirements. The README.md specifies "Python 3.9+" (line 130). If Python 3.10 is specifically required for the build process, the documentation should be updated to clarify this. Otherwise, consider using Python 3.11 to align with the latest stable version and ensure forward compatibility.
| - name: Set up Python 3.10 | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.11' | ||
| python-version: '3.10' |
Copilot
AI
Nov 25, 2025
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.
This change downgrades the Python version from 3.11 to 3.10, which is inconsistent with the project's stated requirements. The README.md specifies "Python 3.9+" (line 130). If Python 3.10 is specifically required for the build process, the documentation should be updated to clarify this. Otherwise, consider using Python 3.11 to align with the latest stable version and ensure forward compatibility.
| # In TwitchIO 3.x, channels from initial_channels should auto-join | ||
| # But we can verify by checking connected channels | ||
| if hasattr(self, 'connected_channels'): | ||
| logger.info(f"Connected channels: {list(self.connected_channels)}") | ||
|
|
||
| # Log available methods for debugging | ||
| logger.debug(f"Bot methods: {[m for m in dir(self) if 'channel' in m.lower() or 'join' in m.lower()]}") | ||
|
|
Copilot
AI
Nov 25, 2025
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.
The modified event_ready method now relies on TwitchIO's automatic channel joining behavior instead of explicitly calling join_channels(). This behavioral change lacks test coverage. Consider adding a test to verify that:
- Channels specified in
initial_channelsare automatically joined - The
connected_channelsattribute is properly populated after the bot is ready - The bot can successfully receive messages from the auto-joined channel
This is especially important given the comment mentions this is a TwitchIO 3.x-specific behavior that should be verified.
✅ Windows Build SuccessfulExecutable: Build Status
Download the artifacts from the workflow run to test before merging. Once merged to |
🐳 Docker Image Built SuccessfullyImage: Test this PR with Docker:docker pull ghcr.io/pladisdev/chat-yapper:pr-8844929
docker run -d \
--name chat-yapper-pr \
-p 8069:8008 \
-e TWITCH_CLIENT_ID=your_id \
-e TWITCH_CLIENT_SECRET=your_secret \
ghcr.io/pladisdev/chat-yapper:pr-8844929Access at: http://localhost:8069 The Docker image will be published to the GitHub Container Registry when merged to |
No description provided.