-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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 build error with blob DB. #2287
Conversation
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
utilities/blob_db/blob_file.cc
Outdated
@@ -7,6 +7,7 @@ | |||
#include <chrono> | |||
#include <cinttypes> | |||
#include <memory> | |||
#include <stdio.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we do #include <cstdio>
instead, so that all header files are c++ style, and the functions provided by stdio go into std
namespace (no globals)?
I see in our code we use both stdio.h
and cstdio
, but it would be good to stick to one type (my personal preference is c++ style header files). (larger change for a later time ... I can take it up).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense. Will update.
@yiwu-arbug updated the pull request - view changes - changes since last import |
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: snprintf is in <stdio.h> and not in namespace std. Closes #2287 Reviewed By: anirbanr-fb Differential Revision: D5054752 Pulled By: yiwu-arbug fbshipit-source-id: 356807ec38f3c7d95951cdb41f31a3d3ae0714d4
Is there a reason that blob_db_test was removed from all the targets? Doesn't look like anything runs that test now. |
@tamird The test is going to fail. I'll fix them soon, but for now I'm disabling it. |
Summary:
snprintf is in <stdio.h> and not in namespace std.
Test Plan:
See if Travis test pass.