Skip to content

Clean up some warnings and errors#1894

Merged
colinator27 merged 5 commits intomasterfrom
warning-error-cleanup
Jan 20, 2025
Merged

Clean up some warnings and errors#1894
colinator27 merged 5 commits intomasterfrom
warning-error-cleanup

Conversation

@colinator27
Copy link
Copy Markdown
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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 27, 2024

@colinator27 colinator27 marked this pull request as ready for review August 27, 2024 16:38
@@ -1,3 +1,5 @@
#pragma warning disable CA1416 // Validate platform compatibility
Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I would prefer it to only be used when absolutely necessary, rather than giving us the opportunity to accidentally write non-cross-platform code where we don't expect it.

@colinator27
Copy link
Copy Markdown
Member Author

Reviews seem pretty much resolved, and I want to take advantage of the warnings when merging the next batch of PRs, so I'll go ahead and merge this for now. For certain style things, we can certainly change them later.

@colinator27 colinator27 merged commit a5c0dc3 into master Jan 20, 2025
@colinator27 colinator27 deleted the warning-error-cleanup branch January 20, 2025 20:31
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