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

Clean up some warnings and errors #1894

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

colinator27
Copy link
Member

@colinator27 colinator27 commented Aug 27, 2024

Description

PR checks have been plagued with tons of warning messages at the end (at least with GitHub's beta "Unchanged files with check annotations"), so this PR aims to get rid of a bunch of them.

Caveats

WebClient was replaced with HttpClient, so there may be slight behavioral differences. I did test the updater, though, and it functioned fine.

Some files now have #pragma directives to disable cross-platform compatibility warnings, but I limited this specifically to GUI, which is already Windows-only, at least as it stands.

Notes

Starting this as a draft to make sure the warnings no longer show up in the build pipeline. This seems to have worked.

Copy link

github-actions bot commented Aug 27, 2024

@colinator27 colinator27 marked this pull request as ready for review August 27, 2024 16:38
UndertaleModCli/CommandOptions/DumpOptions.cs Outdated Show resolved Hide resolved
UndertaleModCli/CommandOptions/InfoOptions.cs Outdated Show resolved Hide resolved
UndertaleModCli/CommandOptions/LoadOptions.cs Outdated Show resolved Hide resolved
UndertaleModCli/CommandOptions/NewOptions.cs Outdated Show resolved Hide resolved
UndertaleModCli/CommandOptions/ReplaceOptions.cs Outdated Show resolved Hide resolved
UndertaleModCli/Program.cs Outdated Show resolved Hide resolved
UndertaleModLib/Scripting/IScriptInterface.cs Outdated Show resolved Hide resolved
UndertaleModTool/MainWindow.xaml.cs Outdated Show resolved Hide resolved
/// <param name="scriptFile">The path to the script file where <see cref="code"/> was executed from.
/// Leave as null, if it wasn't executed from a script file</param>
/// <param name="scriptFile">The path to the script file where <paramref name="code"/> was loaded from.
/// Leave as null, if it wasn't executed from a script file.</param>
private void RunCSharpCode(string code, string scriptFile = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

isnt scriptfile supposed to be a nullable? or is that disabled for the cli project

Comment on lines +323 to +335
public Task ClickableSearchOutput(string title, string query, int resultsCount, IOrderedEnumerable<KeyValuePair<string, List<(int lineNum, string codeLine)>>> resultsDict, bool showInDecompiledView, IOrderedEnumerable<string> failedList = null)
{
throw new NotImplementedException();
}

public Task ClickableSearchOutput(string title, string query, int resultsCount, IDictionary<string, List<(int lineNum, string codeLine)>> resultsDict, bool showInDecompiledView, IEnumerable<string> failedList = null)
{
throw new NotImplementedException();
}

public void ChangeSelection(object newSelection, bool inNewTab = false)
{
}
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the use of these?

@@ -1,3 +1,5 @@
#pragma warning disable CA1416 // Validate platform compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of throwing this into every file, wonder if it makes more sense to put itinto the project.

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.

3 participants