Skip to content

Conversation

@vedgy
Copy link
Contributor

@vedgy vedgy commented Mar 7, 2021

Hopefully the code does not rely on the side effects of the removed call to DBHelper::getLibraryName().

If the call to DBHelper::getLibraryName() is useful, either the variable assignment should be removed or an explaining comment should be added.

@vedgy vedgy mentioned this pull request Mar 7, 2021
@luisangelsm
Copy link
Member

This is in other controllers too, it shouldn't be necessary, so I think we should remove it in all the controllers that don't use the library name and then test the server just in case.

@vedgy vedgy force-pushed the remove-unused-variable branch from dc0c6b3 to f60e176 Compare March 13, 2021 08:19
@vedgy
Copy link
Contributor Author

vedgy commented Mar 13, 2021

we should remove it in all the controllers that don't use the library name

Done. All other controllers use this variable. Only the PageController* source files don't need #include "db_helper.h" after the variable removal. Looks like there is lots of code duplication in the controllers :(

@luisangelsm
Copy link
Member

Looking good, thanks.

@luisangelsm luisangelsm merged commit 6ab5a83 into YACReader:develop Mar 13, 2021
@vedgy vedgy deleted the remove-unused-variable branch March 13, 2021 11:19
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