upgrade: migrate to .NET 8 - #28
Conversation
- Core and GUI target net8.0-windows - Convert GUI project to SDK-style - Remove Costura.Fody and Fody weavers (no longer needed) - Remove supportedRuntime from App.config
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughThe pull request upgrades the project infrastructure from .NET Framework 4.7.2 to .NET 8.0-windows targeting. This includes updating the Core library target framework, converting the GUI project to SDK-style format, removing legacy App.config startup configuration, updating solution project GUIDs, and removing Fody weaver configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| <ApplicationIcon>propeller.ico</ApplicationIcon> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <ApplicationManifest>app.manifest</ApplicationManifest> |
There was a problem hiding this comment.
CRITICAL: Missing app.manifest file
The project references app.manifest but the file does not exist in the project. This will cause build warnings and may lead to runtime issues (e.g., DPI awareness, dependency resolution). Please either add the manifest file or remove the reference.
| <PropertyGroup> | ||
| <TargetFramework>net8.0-windows</TargetFramework> | ||
| <UseWindowsForms>true</UseWindowsForms> | ||
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> |
There was a problem hiding this comment.
MAJOR: GenerateAssemblyInfo set to false without alternative
Setting GenerateAssemblyInfo=false means the assembly will lack important metadata (title, description, configuration, company, product, copyright, trademark, version, etc.) unless you provide a custom AssemblyInfo.cs file. Consider either:
- Setting
GenerateAssemblyInfo=true(recommended for most projects) - Adding a
Properties/AssemblyInfo.csfile with the necessary attributes
This can affect application manifests, ClickOnce deployment, and assembly identification.
Code Review SummaryStatus: 2 Critical Issues | Recommendation: Do not merge - Requires fixes Overview
CRITICAL Issues
Other Observations (not in diff)
Files Reviewed
Recommendations
Compatibility NotesThe code appears compatible with .NET 8.0, but please verify that all P/Invoke signatures and Windows Forms usage are correct. Test thoroughly on target hardware. Reviewed by trinity-large-thinking · 705,042 tokens |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the project to .NET 8.0-windows and adopts the SDK-style project format, removing Fody and Costura dependencies. Feedback identifies a missing "app.manifest" file and a required NuGet reference for "System.Diagnostics.PerformanceCounter" to ensure successful compilation. Additionally, it is recommended to add specific publish properties to the project file to maintain the single-file, 64-bit execution behavior previously handled by Costura.
| <ApplicationIcon>propeller.ico</ApplicationIcon> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <ApplicationManifest>app.manifest</ApplicationManifest> |
There was a problem hiding this comment.
The ApplicationManifest property is set to app.manifest, but this file is not present in the repository or included in this pull request. This will result in a build error (MSB3113: Could not find file 'app.manifest'). Please either include the app.manifest file (which is often required for requesting administrator privileges in hardware-related apps) or remove this property.
| </Compile> | ||
| <Compile Include="LoggingDialog.Designer.cs"> | ||
| <DependentUpon>LoggingDialog.cs</DependentUpon> | ||
| <ProjectReference Include="..\AsusFanControl.Core\AsusFanControl.Core.csproj" /> |
There was a problem hiding this comment.
The project uses System.Diagnostics.PerformanceCounter in Form1.cs. In .NET 8, this class is no longer part of the base framework and requires an explicit NuGet package reference. Without adding this package, the project will fail to compile or throw a runtime exception.
<PackageReference Include="System.Diagnostics.PerformanceCounter" Version="8.0.0" />
<ProjectReference Include="..\AsusFanControl.Core\AsusFanControl.Core.csproj" />
| <Prefer32Bit>false</Prefer32Bit> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <TargetFramework>net8.0-windows</TargetFramework> |
There was a problem hiding this comment.
The pull request description mentions that Costura.Fody was removed because it is no longer needed with .NET 8's native single-file publish. To replicate the standalone behavior of Costura and ensure the application runs as a 64-bit process (which is required to load the 64-bit native AsusWinIO64.dll), it is recommended to add the relevant publish properties to the project file. This ensures that dotnet publish uses these settings by default without requiring manual command-line arguments.
<TargetFramework>net8.0-windows</TargetFramework>
<RuntimeIdentifier>win-x64</RuntimeIdentifier>
<PublishSingleFile>true</PublishSingleFile>
<SelfContained>true</SelfContained>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AsusFanControlGUI/AsusFanControlGUI.csproj`:
- Around line 3-12: The project lacks an explicit PlatformTarget causing AnyCPU
builds to potentially run 32-bit and fail to P/Invoke into the 64-bit
AsusWinIO64.dll; update the AsusFanControlGUI project file to enforce 64-bit by
adding a PlatformTarget of x64 (and optionally set Prefer32Bit to false) inside
the existing PropertyGroup so all builds align with the solution's x64
configuration and P/Invoke to AsusWinIO64.dll from your code (e.g., methods that
call into the native DLL) will succeed.
- Around line 1-12: The project file lacks publish properties for single-file
deployment which are needed to bundle native DLLs (e.g., AsusWinIO64.dll); add
or enable MSBuild properties such as PublishSingleFile=true,
IncludeNativeLibrariesForSelfExtract=true (and optionally SelfContained=true
plus an appropriate RuntimeIdentifier) to the AsusFanControlGUI.csproj
PropertyGroup so the native AsusWinIO64.dll is packaged into the single-file
publish output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a36875c7-1023-4348-9fde-139fbb235124
📒 Files selected for processing (5)
AsusFanControl.Core/AsusFanControl.Core.csprojAsusFanControl.slnAsusFanControlGUI/App.configAsusFanControlGUI/AsusFanControlGUI.csprojAsusFanControlGUI/FodyWeavers.xml
💤 Files with no reviewable changes (2)
- AsusFanControlGUI/FodyWeavers.xml
- AsusFanControlGUI/App.config
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | ||
| <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | ||
| <ProjectGuid>{42CC78B6-E3BB-4092-A423-A4EC20FB3C11}</ProjectGuid> | ||
| <OutputType>WinExe</OutputType> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion> | ||
| <FileAlignment>512</FileAlignment> | ||
| <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> | ||
| <Deterministic>true</Deterministic> | ||
| <RuntimeIdentifier>win-x64</RuntimeIdentifier> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> | ||
| <PlatformTarget>AnyCPU</PlatformTarget> | ||
| <DebugSymbols>true</DebugSymbols> | ||
| <DebugType>full</DebugType> | ||
| <Optimize>false</Optimize> | ||
| <OutputPath>bin\Debug\</OutputPath> | ||
| <DefineConstants>DEBUG;TRACE</DefineConstants> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <WarningLevel>4</WarningLevel> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> | ||
| <PlatformTarget>AnyCPU</PlatformTarget> | ||
| <DebugType>none</DebugType> | ||
| <Optimize>true</Optimize> | ||
| <OutputPath>bin\Release\</OutputPath> | ||
| <DefineConstants>TRACE</DefineConstants> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <WarningLevel>4</WarningLevel> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'"> | ||
| <DebugSymbols>true</DebugSymbols> | ||
| <OutputPath>..\bin\x64\Debug\</OutputPath> | ||
| <DefineConstants>DEBUG;TRACE</DefineConstants> | ||
| <DebugType>full</DebugType> | ||
| <PlatformTarget>x64</PlatformTarget> | ||
| <LangVersion>7.3</LangVersion> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <Prefer32Bit>false</Prefer32Bit> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'"> | ||
| <OutputPath>..\bin\x64\Release\</OutputPath> | ||
| <DefineConstants>TRACE</DefineConstants> | ||
| <Optimize>true</Optimize> | ||
| <DebugType>none</DebugType> | ||
| <PlatformTarget>x64</PlatformTarget> | ||
| <LangVersion>7.3</LangVersion> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <Prefer32Bit>false</Prefer32Bit> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <TargetFramework>net8.0-windows</TargetFramework> | ||
| <UseWindowsForms>true</UseWindowsForms> | ||
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> | ||
| <ApplicationIcon>propeller.ico</ApplicationIcon> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <ApplicationManifest>app.manifest</ApplicationManifest> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding publish configuration for single-file deployment.
The PR description mentions single-file publish as the replacement for Costura.Fody. For the native AsusWinIO64.dll to be bundled in a single-file deployment, you'll need IncludeNativeLibrariesForSelfExtract. If you intend to configure this via CLI arguments at publish time, this is fine to ignore.
📦 Optional: Add single-file publish properties
<PropertyGroup>
<OutputType>WinExe</OutputType>
<TargetFramework>net8.0-windows</TargetFramework>
<UseWindowsForms>true</UseWindowsForms>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<ApplicationIcon>propeller.ico</ApplicationIcon>
<AssemblyName>AsusFanControl</AssemblyName>
<RootNamespace>AsusFanControlGUI</RootNamespace>
<ApplicationManifest>app.manifest</ApplicationManifest>
+ <!-- Single-file publish settings (alternative: use dotnet publish -p:PublishSingleFile=true) -->
+ <PublishSingleFile>true</PublishSingleFile>
+ <SelfContained>true</SelfContained>
+ <IncludeNativeLibrariesForSelfExtract>true</IncludeNativeLibrariesForSelfExtract>
</PropertyGroup>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AsusFanControlGUI/AsusFanControlGUI.csproj` around lines 1 - 12, The project
file lacks publish properties for single-file deployment which are needed to
bundle native DLLs (e.g., AsusWinIO64.dll); add or enable MSBuild properties
such as PublishSingleFile=true, IncludeNativeLibrariesForSelfExtract=true (and
optionally SelfContained=true plus an appropriate RuntimeIdentifier) to the
AsusFanControlGUI.csproj PropertyGroup so the native AsusWinIO64.dll is packaged
into the single-file publish output.
| <PropertyGroup> | ||
| <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | ||
| <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | ||
| <ProjectGuid>{42CC78B6-E3BB-4092-A423-A4EC20FB3C11}</ProjectGuid> | ||
| <OutputType>WinExe</OutputType> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion> | ||
| <FileAlignment>512</FileAlignment> | ||
| <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> | ||
| <Deterministic>true</Deterministic> | ||
| <RuntimeIdentifier>win-x64</RuntimeIdentifier> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> | ||
| <PlatformTarget>AnyCPU</PlatformTarget> | ||
| <DebugSymbols>true</DebugSymbols> | ||
| <DebugType>full</DebugType> | ||
| <Optimize>false</Optimize> | ||
| <OutputPath>bin\Debug\</OutputPath> | ||
| <DefineConstants>DEBUG;TRACE</DefineConstants> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <WarningLevel>4</WarningLevel> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> | ||
| <PlatformTarget>AnyCPU</PlatformTarget> | ||
| <DebugType>none</DebugType> | ||
| <Optimize>true</Optimize> | ||
| <OutputPath>bin\Release\</OutputPath> | ||
| <DefineConstants>TRACE</DefineConstants> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <WarningLevel>4</WarningLevel> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'"> | ||
| <DebugSymbols>true</DebugSymbols> | ||
| <OutputPath>..\bin\x64\Debug\</OutputPath> | ||
| <DefineConstants>DEBUG;TRACE</DefineConstants> | ||
| <DebugType>full</DebugType> | ||
| <PlatformTarget>x64</PlatformTarget> | ||
| <LangVersion>7.3</LangVersion> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <Prefer32Bit>false</Prefer32Bit> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'"> | ||
| <OutputPath>..\bin\x64\Release\</OutputPath> | ||
| <DefineConstants>TRACE</DefineConstants> | ||
| <Optimize>true</Optimize> | ||
| <DebugType>none</DebugType> | ||
| <PlatformTarget>x64</PlatformTarget> | ||
| <LangVersion>7.3</LangVersion> | ||
| <ErrorReport>prompt</ErrorReport> | ||
| <Prefer32Bit>false</Prefer32Bit> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <TargetFramework>net8.0-windows</TargetFramework> | ||
| <UseWindowsForms>true</UseWindowsForms> | ||
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> | ||
| <ApplicationIcon>propeller.ico</ApplicationIcon> | ||
| <AssemblyName>AsusFanControl</AssemblyName> | ||
| <RootNamespace>AsusFanControlGUI</RootNamespace> | ||
| <ApplicationManifest>app.manifest</ApplicationManifest> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
Add explicit platform target for 64-bit native DLL compatibility.
The project depends on AsusWinIO64.dll (64-bit native DLL), but no PlatformTarget is specified. SDK-style projects default to AnyCPU, which could allow the application to run as 32-bit on some systems, causing P/Invoke failures. The solution file expects x64 builds (lines 20-23), but without this property the project won't have a matching configuration.
🛡️ Proposed fix to enforce x64 platform
<PropertyGroup>
<OutputType>WinExe</OutputType>
<TargetFramework>net8.0-windows</TargetFramework>
+ <PlatformTarget>x64</PlatformTarget>
<UseWindowsForms>true</UseWindowsForms>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<ApplicationIcon>propeller.ico</ApplicationIcon>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PropertyGroup> | |
| <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | |
| <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | |
| <ProjectGuid>{42CC78B6-E3BB-4092-A423-A4EC20FB3C11}</ProjectGuid> | |
| <OutputType>WinExe</OutputType> | |
| <RootNamespace>AsusFanControlGUI</RootNamespace> | |
| <AssemblyName>AsusFanControl</AssemblyName> | |
| <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion> | |
| <FileAlignment>512</FileAlignment> | |
| <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects> | |
| <Deterministic>true</Deterministic> | |
| <RuntimeIdentifier>win-x64</RuntimeIdentifier> | |
| </PropertyGroup> | |
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> | |
| <PlatformTarget>AnyCPU</PlatformTarget> | |
| <DebugSymbols>true</DebugSymbols> | |
| <DebugType>full</DebugType> | |
| <Optimize>false</Optimize> | |
| <OutputPath>bin\Debug\</OutputPath> | |
| <DefineConstants>DEBUG;TRACE</DefineConstants> | |
| <ErrorReport>prompt</ErrorReport> | |
| <WarningLevel>4</WarningLevel> | |
| </PropertyGroup> | |
| <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> | |
| <PlatformTarget>AnyCPU</PlatformTarget> | |
| <DebugType>none</DebugType> | |
| <Optimize>true</Optimize> | |
| <OutputPath>bin\Release\</OutputPath> | |
| <DefineConstants>TRACE</DefineConstants> | |
| <ErrorReport>prompt</ErrorReport> | |
| <WarningLevel>4</WarningLevel> | |
| </PropertyGroup> | |
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'"> | |
| <DebugSymbols>true</DebugSymbols> | |
| <OutputPath>..\bin\x64\Debug\</OutputPath> | |
| <DefineConstants>DEBUG;TRACE</DefineConstants> | |
| <DebugType>full</DebugType> | |
| <PlatformTarget>x64</PlatformTarget> | |
| <LangVersion>7.3</LangVersion> | |
| <ErrorReport>prompt</ErrorReport> | |
| <Prefer32Bit>false</Prefer32Bit> | |
| </PropertyGroup> | |
| <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'"> | |
| <OutputPath>..\bin\x64\Release\</OutputPath> | |
| <DefineConstants>TRACE</DefineConstants> | |
| <Optimize>true</Optimize> | |
| <DebugType>none</DebugType> | |
| <PlatformTarget>x64</PlatformTarget> | |
| <LangVersion>7.3</LangVersion> | |
| <ErrorReport>prompt</ErrorReport> | |
| <Prefer32Bit>false</Prefer32Bit> | |
| </PropertyGroup> | |
| <PropertyGroup> | |
| <TargetFramework>net8.0-windows</TargetFramework> | |
| <UseWindowsForms>true</UseWindowsForms> | |
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> | |
| <ApplicationIcon>propeller.ico</ApplicationIcon> | |
| <AssemblyName>AsusFanControl</AssemblyName> | |
| <RootNamespace>AsusFanControlGUI</RootNamespace> | |
| <ApplicationManifest>app.manifest</ApplicationManifest> | |
| </PropertyGroup> | |
| <PropertyGroup> | |
| <OutputType>WinExe</OutputType> | |
| <TargetFramework>net8.0-windows</TargetFramework> | |
| <PlatformTarget>x64</PlatformTarget> | |
| <UseWindowsForms>true</UseWindowsForms> | |
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> | |
| <ApplicationIcon>propeller.ico</ApplicationIcon> | |
| <AssemblyName>AsusFanControl</AssemblyName> | |
| <RootNamespace>AsusFanControlGUI</RootNamespace> | |
| <ApplicationManifest>app.manifest</ApplicationManifest> | |
| </PropertyGroup> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AsusFanControlGUI/AsusFanControlGUI.csproj` around lines 3 - 12, The project
lacks an explicit PlatformTarget causing AnyCPU builds to potentially run 32-bit
and fail to P/Invoke into the 64-bit AsusWinIO64.dll; update the
AsusFanControlGUI project file to enforce 64-bit by adding a PlatformTarget of
x64 (and optionally set Prefer32Bit to false) inside the existing PropertyGroup
so all builds align with the solution's x64 configuration and P/Invoke to
AsusWinIO64.dll from your code (e.g., methods that call into the native DLL)
will succeed.
1 participant
Summary
Summary by CodeRabbit