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

Fix warnings part 1 #3251

Merged
merged 9 commits into from
Feb 5, 2025
Merged

Conversation

Saibamen
Copy link
Contributor

@Saibamen Saibamen commented Feb 4, 2025

Remove unuses usings. Remove redundant (). Fix one CS4014. Add missing braces for foreach. Remove redundant else. Fix other warnings

…g braces for foreach. Remove redundant else. Fix other warnings
@Saibamen Saibamen changed the title Remove unuses usings. Remove redundant (). Fix one CS4014. Add missin… Fix warnings part 1 Feb 4, 2025
@Saibamen
Copy link
Contributor Author

Saibamen commented Feb 5, 2025

@marticliment Can you please review this? I'm still fixing merge conflicts.

@marticliment marticliment merged commit 550a8b3 into marticliment:main Feb 5, 2025
2 checks passed
@marticliment
Copy link
Owner

Hey @Saibamen,

I really appreciate the effort put into improving the codebase, but please do not push PRs with style changes (e.g. replacing new() with [], adding/removing optional brackets, spaces after the if clause, etc.), since they are very time-consuming to review, and don't offer a real benefit on code performance or efficiency.
However, the efficiency changes (IsEmpty, Count == 0) are great.

Thanks for the effort!
Martí

@Saibamen Saibamen deleted the fix_warnings_part1 branch February 6, 2025 16:28
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