Repository navigation
Add native build verification script - #3
Conversation
Summary of ChangesHello @Calvindd2f, 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 introduces a fundamental architectural change to the OfficeScrubC2R module. Instead of relying on extensive PowerShell scripting and dynamic C# compilation, the module now acts as a streamlined wrapper for a pre-compiled native C# DLL. This refactoring aims to simplify the module's structure, improve maintainability, and ensure robust interaction with native Windows APIs. The change also includes a new verification script to confirm the native build process across different PowerShell environments. Highlights
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.
Code Review
This pull request introduces a significant and positive architectural refactoring, moving the core logic into a native C# DLL and simplifying the PowerShell module into a lightweight wrapper. This change should improve performance and maintainability. The addition of the Verify-NativeBuild.ps1 script is excellent, as it provides a robust way to validate the native compilation process across different .NET environments.
My review includes a couple of suggestions for the new verification script to enhance its robustness and error handling. Overall, this is a high-quality contribution that modernizes the project's architecture.
| "@ | Set-Content -Path $projectPath -Encoding UTF8 | ||
|
|
||
| Write-Host "Building .NET SDK assembly targeting $Framework" -ForegroundColor Cyan | ||
| dotnet build $projectPath -nologo -clp:NoSummary -p:GenerateAssemblyInfo=false | Write-Host |
There was a problem hiding this comment.
Piping the output of dotnet build directly to Write-Host merges the standard output and error streams into the information stream. This can make it harder to spot build errors, as they will not be displayed as PowerShell errors. Removing the pipe to Write-Host will allow PowerShell to handle the output streams correctly, rendering errors in red as expected.
dotnet build $projectPath -nologo -clp:NoSummary -p:GenerateAssemblyInfo=false
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $frameworkCandidates = @( | ||
| Join-Path $env:WINDIR 'Microsoft.NET\\Framework64\\v4.0.30319\\csc.exe'), | ||
| Join-Path $env:WINDIR 'Microsoft.NET\\Framework\\v4.0.30319\\csc.exe' | ||
| ) | Where-Object { $_ -and (Test-Path $_) } |
There was a problem hiding this comment.
Resolve syntax error in .NET Framework compiler search
The new verification script won't parse because the $frameworkCandidates array is initialized with an extra closing parenthesis (Join-Path ...),), which causes PowerShell to throw before any logic runs. As written, invoking tests/Verify-NativeBuild.ps1 fails immediately instead of checking the native build.
Useful? React with 👍 / 👎.
Summary
Testing
Codex Task