Skip to content

Conversation

@BIGbadEL
Copy link
Collaborator

No description provided.

@BIGbadEL BIGbadEL added the merge asap This should be merged to master as soon as possible label Jul 25, 2019
@BIGbadEL BIGbadEL requested a review from Blinkuu July 25, 2019 13:07
@BIGbadEL BIGbadEL self-assigned this Jul 25, 2019
${CMAKE_CURRENT_SOURCE_DIR}/src/core/platform/opengl/buffers/opengl_vertex_array.cpp
${CMAKE_CURRENT_SOURCE_DIR}/src/core/platform/opengl/shaders/opengl_shader.cpp
${CMAKE_CURRENT_SOURCE_DIR}/src/core/utils/file_manager.cpp
src/core/utils/file_manager/file_manager.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

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

add ${CMAKE_CURRENT_SOURCE_DIR}/

CORE_LOG_INFO("[FILE_MANAGER] Error: Windows implementation is not done yet! Using slower reading method!");
ReadSmallFile(filePath);
// CORE_LOG_INFO("[FILE_MANAGER] Error: Windows implementation is not done yet! Using slower reading method!");
// ReadSmallFile(filePath);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove comments

std::streampos begin,end;
std::ifstream file(filePath, std::ios::in);
char* buffer;
if(file.is_open()){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you do some research on the performance of this code?

@@ -19,7 +19,7 @@ namespace Bald::Utils {
* ENUM which determines size of file and therefor methods, which will be used to read it
*/
enum class Size : char {
SMALL_FILE, BIG_FILE
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to have fixed convention for enum naming

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

@piotrmoszkowicz piotrmoszkowicz requested review from piotrmoszkowicz and removed request for piotrmoszkowicz December 2, 2019 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge asap This should be merged to master as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants