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 some compiler warnings #196

Merged
merged 6 commits into from
Mar 9, 2021

Conversation

vedgy
Copy link
Contributor

@vedgy vedgy commented Feb 6, 2021

See the commit messages for details.

@@ -33,7 +31,6 @@ void PageControllerV2::service(HttpRequest &request, HttpResponse &response)
bool remote = path.endsWith("remote");

QStringList pathElements = path.split('/');
QString libraryName = DBHelper::getLibraryName(pathElements.at(2).toInt());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and the identical change in PageController::service() are the ones I'm not sure about. If the call to DBHelper::getLibraryName() is useful here, either the variable assignment should be removed or an explaining comment should be added.

@vedgy vedgy force-pushed the fix-some-compiler-warnings branch from d022128 to 1e8db7b Compare February 11, 2021 16:13
@selmf
Copy link
Member

selmf commented Mar 7, 2021

From a first glance, this is a bit of a mixed bag. A good amount of these are no-brainers and can be easily merged but there are some commit thrown in (like the pagecontroller) that need closer examination which I can't provide given that I don't use YACReader for iOS.
@vedgy can I trouble you to put the commits that need examination into a separate PR? I'm currently working on something that requires the removal of deprecated api usage and it would be stupid to duplicate your work.

@vedgy
Copy link
Contributor Author

vedgy commented Mar 7, 2021

I think the only risky commit here is the PageController one. I can move that in a separate pull request.

I'm currently working on something that requires the removal of deprecated api usage and it would be stupid to duplicate your work.

I've also ported away from some deprecated Qt API in #206, so you may want to look at it as well.

@vedgy vedgy force-pushed the fix-some-compiler-warnings branch from 1e8db7b to 20e9baa Compare March 7, 2021 13:29
vedgy added 6 commits March 7, 2021 15:30
This change gets rid of some GCC's -Wdeprecated-declarations warnings.
The conversions prevented return value optimization and caused a
-Wreturn-std-move Clang warning.
This change gets rid of a few GCC's -Wdeprecated-declarations warnings.
Moving the initialization of defaultTexture out of the member
initializer list gets rid of a GCC's -Wreorder warning.

Initialize other texture pointers to improve safety and consistency.
@vedgy vedgy force-pushed the fix-some-compiler-warnings branch from 20e9baa to 629eb26 Compare March 7, 2021 13:30
@vedgy
Copy link
Contributor Author

vedgy commented Mar 7, 2021

Split the PageController commit into #231.

@selmf selmf merged commit 21c3bda into YACReader:develop Mar 9, 2021
@vedgy vedgy deleted the fix-some-compiler-warnings branch March 11, 2021 13:35
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.

2 participants