Upgrade to .NET 10 #672
Upgrade to .NET 10 #672
Conversation
Summary of ChangesHello @sven-n, 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 a significant infrastructure upgrade, migrating the entire codebase to .NET 10. This transition is accompanied by the adoption of Centralized Package Management to simplify dependency handling. A major architectural improvement involves extracting shared web functionalities into a new MUnique.OpenMU.Web.Shared project to house common web components and utilities, reducing code duplication and improving maintainability for the web frontends. Additionally, Docker build configurations have been updated to reflect the new .NET version, and a targeted fix has been applied to address a static web asset issue encountered during the upgrade process. 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
|
![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 is a significant step forward, upgrading the project to .NET 10, centralizing package management, and refactoring shared web components. These are all excellent improvements for the project's maintainability and future-proofing. The workaround for the .NET 10 SDK bug is well-documented and correctly implemented.
I've found a few issues that need attention. There are several duplicate package definitions in the new Directory.Packages.props file, which will likely cause build failures. Additionally, some necessary scripts and stylesheets for Blazored.Modal appear to be commented out, which could break modal dialog functionality. I've also included a suggestion to improve maintainability by moving an inline script to a separate file. Please see my detailed comments.
src/Web/Shared/Exports.cs
Outdated
| //yield return "_content/Blazored.Modal/blazored.modal.js"; | ||
| yield return "_content/BlazorInputFile/inputfile.js"; | ||
| } | ||
| } | ||
|
|
||
| private static IEnumerable<string> SharedStylesheets | ||
| { | ||
| get | ||
| { | ||
| yield return "_content/Blazored.Typeahead/blazored-typeahead.css"; | ||
| //yield return "_content/Blazored.Modal/blazored-modal.css"; |
There was a problem hiding this comment.
The scripts and stylesheets for Blazored.Modal are commented out. This library is used in several places for modal dialogs, and without its JavaScript and CSS files, the modals will not function correctly. These lines should be uncommented to ensure modals work as expected.
yield return "_content/Blazored.Typeahead/blazored-typeahead.js";
yield return "_content/Blazored.Modal/blazored.modal.js";
yield return "_content/BlazorInputFile/inputfile.js";
}
}
private static IEnumerable<string> SharedStylesheets
{
get
{
yield return "_content/Blazored.Typeahead/blazored-typeahead.css";
yield return "_content/Blazored.Modal/blazored-modal.css";| <script> | ||
| // plain javascript to make JS Interop easier | ||
| function GetMapHostBoundingClientRect() { | ||
| return document.getElementsByClassName("map-host")[0].getBoundingClientRect(); | ||
| } | ||
| </script> |
There was a problem hiding this comment.
Placing JavaScript directly inside a .razor file is generally not recommended as it can make the code harder to maintain, debug, and reuse. It's better to keep your JavaScript in separate .js files.
I suggest moving this script to its own file, for example, wwwroot/js/map-editor.js within the MUnique.OpenMU.Web.Shared project, and then including it through the Exports.cs file. This will improve code organization and allow the browser to cache the script file.
Tasks:
MUnique.OpenMU.Web.Sharedso that static web assets aren't copied around a dozens of timesGameServer.HostandAdminPanel.Hostis affected by theWeb.Sharedproject changeThe upgrade involves a workaround for the following issue in the .NET 10 SDK: dotnet/sdk#52089