-
Notifications
You must be signed in to change notification settings - Fork 1
feat(tools): enhance map/replay managers and refine tools sidebar ui #10
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
✅ Deploy Preview for generalshub ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughCI/CD pipeline enhancements including improved error handling for token injection (PowerShell script now exits with error code 1 on missing token, uses regex-based replacements, and validates post-injection), earlier token injection in Windows/Linux build flows, Markdown-linked assets in release notes, and .env file copying for local debugging in the Windows project. Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
306-309: Remove duplicate Inject Secrets step.This Inject Secrets step duplicates the earlier injection at lines 288-291. The earlier injection (before Build Projects) is correctly placed and sufficient. Remove this duplicate to avoid redundant script execution and workflow inefficiency.
🔎 Proposed fix
Remove lines 306-309 entirely, as the token is already injected earlier at lines 288-291:
- - name: Inject Secrets - shell: pwsh - run: | - ./.github/scripts/inject-token.ps1 -Token "${{ secrets.UPLOADTHING_TOKEN }}" -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/inject-token.ps1.github/workflows/ci.yml.github/workflows/release.ymlGenHub/GenHub.Windows/GenHub.Windows.csproj
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Windows
- GitHub Check: Build Linux
🔇 Additional comments (7)
GenHub/GenHub.Windows/GenHub.Windows.csproj (1)
32-38: LGTM! Local debugging support properly implemented.The conditional ItemGroup correctly copies the
.envfile from the repository root to the output directory only when it exists, usingPreserveNewestto avoid unnecessary updates. This provides convenient local debugging support without affecting production builds..github/workflows/release.yml (1)
224-225: LGTM! Enhanced user experience with clickable asset links.The Markdown-formatted links improve the release notes by making assets directly clickable, using proper GitHub context variables for dynamic URL generation.
.github/scripts/inject-token.ps1 (3)
6-7: LGTM! Proper error handling for missing token.Correctly exits with code 1 when the token is missing, ensuring the CI/CD pipeline fails fast rather than continuing with invalid credentials.
30-33: LGTM! More robust placeholder replacement.The regex-based replacement correctly handles whitespace variations in the placeholder patterns, making the injection more resilient to code formatting changes.
35-38: LGTM! Reasonable validation check.The post-injection validation ensures that hex byte arrays were successfully injected. While the check is simple (looking for "0x"), it's appropriate for catching obvious injection failures.
.github/workflows/ci.yml (2)
126-129: LGTM! Token injection properly placed before build.The Inject Secrets step is correctly positioned after build info extraction and before the build step, ensuring the token is available when needed.
288-291: LGTM! Token injection properly placed before build.The Inject Secrets step is correctly positioned after build info extraction and before the build step, ensuring the token is available when needed.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.