-
-
Notifications
You must be signed in to change notification settings - Fork 129
VS Code extension: fix/update templates based on ps1-bare-metal #1970
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
base: main
Are you sure you want to change the base?
Conversation
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
tools/vscode-extension/templates/bare-metal/cmake-cube/src/main.c (1)
195-197
: LGTM! Cleaner DMA channel enabling with helper macro.The consolidated approach using
DMA_DPCR_CH_ENABLE()
is more maintainable than manual bit shifts. The|= 0
pattern is unconventional but harmless—you could simplify toDMA_DPCR |= DMA_DPCR_CH_ENABLE(DMA_GPU) | DMA_DPCR_CH_ENABLE(DMA_OTC);
if preferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tools/vscode-extension/templates/bare-metal/cmake-cube/CMakeLists.txt
(1 hunks)tools/vscode-extension/templates/bare-metal/cmake-cube/src/main.c
(2 hunks)tools/vscode-extension/templates/bare-metal/empty-cmake/CMakeLists.txt
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeScene Code Health Review (main)
tools/vscode-extension/templates/bare-metal/cmake-cube/src/main.c
[warning] 195-197: ❌ Getting worse: Complex Method
main already has high cyclomatic complexity, and now it increases in Lines of Code from 87 to 88. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
⏰ 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). (3)
- GitHub Check: macos-intel-build-and-test
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: toolchain
🔇 Additional comments (8)
tools/vscode-extension/templates/bare-metal/cmake-cube/src/main.c (1)
33-33
: LGTM! API naming update aligns with ps1-bare-metal.The change from
COP0_SR
toCOP0_STATUS
andCOP0_SR_CU2
toCOP0_STATUS_CU2
reflects updated naming conventions in the ps1-bare-metal library, improving code clarity.tools/vscode-extension/templates/bare-metal/empty-cmake/CMakeLists.txt (3)
28-36
: LGTM! Library sources updated for ps1-bare-metal compatibility.The addition of
clz.s
,string.s
, andcache.s
alongside the removal of obsolete sources aligns the template with upstream ps1-bare-metal changes.
44-44
: LGTM! Default linking simplifies executable configuration.The
link_libraries(common)
directive ensures the common library is automatically linked to all executables, reducing boilerplate in subsequent target definitions.
46-50
: LGTM! Macro-driven executable creation reduces boilerplate.The
addPS1Executable()
macro encapsulates executable creation and PS1 format conversion, eliminating the need for manualadd_custom_command
post-build steps. This modernizes the build flow and improves maintainability.tools/vscode-extension/templates/bare-metal/cmake-cube/CMakeLists.txt (4)
28-36
: LGTM! Library sources synchronized with ps1-bare-metal.Identical to the
empty-cmake
template, these source updates maintain compatibility with upstream ps1-bare-metal changes.
44-44
: LGTM! Consistent default linking across templates.The
link_libraries(common)
directive matches the approach in theempty-cmake
template, ensuring consistency.
46-56
: LGTM! Multi-source executable with addPS1Executable.The macro correctly handles multiple source files, demonstrating its flexibility beyond the single-file usage in the
empty-cmake
template.
58-64
: LGTM! Updated documentation reflects macro-based asset embedding.The comments accurately reference the
convertImage()
andaddBinaryFile()
macros, guiding users on the modernized asset embedding workflow.
Quick fixes for the
empty-cmake
andcmake-cube
templates provided by the PSX.Dev VS Code extension, which no longer compiled due to some minor but breaking changes in the ps1-bare-metal repository they depend on. I haven't bothered to bump the extension's version number as I already did that as part of #1948, which is still unmerged.