-
Notifications
You must be signed in to change notification settings - Fork 21
fix: hide stream latency on clips #123
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
Conversation
Summary of ChangesHello @igorovh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on two areas: enhancing the build and packaging script for better code quality and maintainability, and refining the user interface by ensuring that stream latency information does not appear on chat clips, which improves the overall viewing experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request refactors the packaging script and fixes an issue with stream latency display on clips. The script refactoring improves code clarity but introduces a minor regression in handling process exit codes. The latency fix has a potential logic bug where it might prematurely exit a loop. My review includes suggestions to address these two points.
src/platforms/kick/modules/stream-latency/stream-latency.module.tsx
Outdated
Show resolved
Hide resolved
scripts/pack.ts
Outdated
| if ((await buildProc.exited) !== 0) { | ||
| console.error("Build failed"); | ||
| process.exit(1); | ||
| } |
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 refactoring has introduced a small regression. By hardcoding the exit code to 1, we lose the specific exit code from the build process, which can be useful for debugging. The previous implementation was better in this regard as it propagated the actual exit code. I'd suggest reverting to a similar pattern to preserve this information.
const buildStatus = await buildProc.exited;
if (buildStatus !== 0) {
console.error("Build failed");
process.exit(buildStatus);
}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 aims to hide the stream latency indicator when viewing clips on the Kick platform, while also refactoring the build script for improved code quality.
- Added check to skip rendering stream latency component on chat rooms with the
--chat-clipclass - Refactored the
pack.tsscript with clearer naming and more modern async patterns - Simplified the build process flow and improved code readability
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/platforms/kick/modules/stream-latency/stream-latency.module.tsx |
Added conditional check to skip stream latency display on clip chat rooms |
scripts/pack.ts |
Refactored archive creation logic with improved naming, modernized async patterns, and restructured code flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.updateInterval = setInterval(() => this.updateLatency(), 1000); | ||
|
|
||
| for (const chatRoom of elements) { | ||
| if (chatRoom.className.includes("--chat-clip")) return; |
Copilot
AI
Dec 29, 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.
Using 'return' here will exit the entire function when the first chat room has the '--chat-clip' class, preventing processing of subsequent chat rooms in the loop. This should use 'continue' instead to skip only the current iteration and process remaining chat rooms.
| if (chatRoom.className.includes("--chat-clip")) return; | |
| if (chatRoom.className.includes("--chat-clip")) continue; |
| await archiveDirectory("dist", buildZipPath); | ||
| console.log(`Created ${buildZipPath}`); | ||
|
|
||
| const sourceZipPath = path.join("dist", `source-${version}.zip`); | ||
| await archiveDirectory(".", sourceZipPath, IGNORED_DIRS); |
Copilot
AI
Dec 29, 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 build zip file created on line 47 will be included in the source zip created on line 51. The original implementation excluded the build zip file from the source archive. This means the source zip will now contain the build zip, which is likely unintended and will significantly increase the source archive size.
…e.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
No description provided.