Skip to content
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

Feature/ilc #43

Merged
merged 45 commits into from
Mar 10, 2025
Merged

Feature/ilc #43

merged 45 commits into from
Mar 10, 2025

Conversation

valentinbreiz
Copy link
Owner

No description provided.

@zarlo
Copy link
Collaborator

zarlo commented Mar 4, 2025

This does not need to be done in this PR, but in #35 i have a nuget package that has a linker for windows and im using LdPath to get the path, im happy to change it to CppLinker

@Guillermo-Santos
Copy link
Collaborator

Using the dotnet bat file worked for me, but if there are other ways then it may be useful.

@kumja1 kumja1 requested a review from Copilot March 9, 2025 04:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR implements feature enhancements for the ILC by adding support for an output path, refactoring plug reference handling, and updating diagnostic filtering.

  • Updated ProjectInfo and PlugInfo models to better reflect plug references.
  • Modified build task and patch command logic to include an output path.
  • Refactored diagnostic filtering in the code fix provider and simplified workflow configurations.

Reviewed Changes

File Description
src/Liquip.Patcher.Analyzer.CodeFixes/Models/ProjectInfo.cs Added an explicit readonly field for PlugReferences that duplicates the record’s parameter.
src/Liquip.Patcher.Build/Tasks/PatcherTask.cs Introduced a new OutputPath property and updated command line arguments and logging accordingly.
src/Liquip.Patcher.Analyzer/Models/PlugInfo.cs Renamed the second parameter from PlugSymbol to PluggedSymbol and exposed readonly properties.
src/Liquip.Patcher.Analyzer.CodeFixes/PatcherCodeFixProvider.cs Simplified diagnostic filtering logic and refactored the syntax reference handling.
.github/workflows/dotnet.yml Simplified workflow configuration by removing the matrix and hardcoding the dotnet version.
src/Liquip.Patcher/PatchCommand.cs Added a new command option for output and modified assembly reading and output file construction.
src/Liquip.Patcher.Analyzer/PatcherAnalyzer.cs Updated diagnostic identifier and adjusted references to the plug symbol accordingly.
Extensions & EnumerableExtensions files Removed unused using directives, renamed a class for clarity, and deleted the redundant enumerable extension.

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Liquip.Patcher/PatchCommand.cs:59

  • The output file's directory is derived from 'settings.OutputPath' while the filename is based on 'settings.TargetAssembly'. Verify if this mixed usage is intended or consider using 'settings.OutputPath' consistently to define the full output path.
string? outputPath = Path.Combine(Path.GetDirectoryName(settings.OutputPath)!, Path.GetFileNameWithoutExtension(settings.TargetAssembly) + "_patched.dll");

@Guillermo-Santos
Copy link
Collaborator

should we change to a nuget patckage reference instead of calling the target directly?

@kumja1
Copy link
Collaborator

kumja1 commented Mar 9, 2025

@valentinbreiz This is ready to merge

@valentinbreiz valentinbreiz merged commit 66379b9 into valentinbreiz:main Mar 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants