Clean up codebase and fix bug #1575
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The mechanism that both pack updating and Modrinth overrides use utilize std::filesystem::copy, which with GCC's libstdc++ has a bug on Windows where `overwrite_existing` isn't obeyed. In addition, made it clear what `overrideFolder` does by renaming it and rewriting an error message.
from -Wc++20-compat
| fs::create_directory(fullDstPath / relativeChild); | ||
| if (entry.is_regular_file()) | ||
| { | ||
| fs::path childDst = fullDstPath / relativeChild; |
There was a problem hiding this comment.
why is this declared here instead of higher up
There was a problem hiding this comment.
it's declared inside the if's scope, since it's not used outside of that scope, why declare it anywhere else?
|
You should signoff your commits with -s |
LennyMcLennington
requested changes
May 7, 2023
| @@ -69,6 +69,7 @@ bool JavaWizardPage::validatePage() | |||
| case JavaSettingsWidget::ValidationStatus::AllOK: | |||
| { | |||
| settings->set("JavaPath", m_java_widget->javaPath()); | |||
| return true; | |||
There was a problem hiding this comment.
this looks like it should fall through
for FileSystem.cpp: Instead of checking if Linux or FreeBSD, check if its not Windows and not OSX. Chances are other operating systems run a DE that adheres to the XDG Desktop standard (.desktop). The check isn't good enough anyways since alternative shells for Windows exist, it will never be an accurate check. In any case this function is unused. WorldListPage.cpp: Redo confusing switch statement plagued with fall throughs, now well defined. LaunchController.cpp: Remove cringe. Also fix warning and make the unimplemented case(s) more explicit. VersionProxyModel.cpp: Add fallthrough for warning suppression. WorldListPage.cpp: redo `mceditState` TranslationsModel.cpp: Move up definition of `column` variable to when it is needed, clear up switch cases FlameInstanceCreationTask.cpp: Fallthrough intentionally SkinUpload.cpp: Make `getVariant` ResourcePack.cpp: Add new values for 1.19.3+ meta/Index.cpp: Make clear switch statement behavior JavaWizardPage.cpp: Fix case fallthrough Yggdrasil.cpp: Fix case fallthrough AccountList.cpp: Fix case fallthrough, WinDarkmode.cpp: Add an explanation and fix warnings due to FARPROC casts. Signed-off-by: jdp_ <42700985+jdpatdiscord@users.noreply.github.com>
|
LGTM!!!!!!!!!!!!!!! |
7EAC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit details for more information