To use the same Logger API we have on native rn.
Not sure what will be the best place to put this file, but I think it's better to separate and avoid adding "web" implementation in native/ directory.
Details
Details
- Reviewers
michal tomek • jon - Commits
- rCOMM147eeb078d0a: [web] Add web Logger implementation
- Check if clang format works
- Check if Logger works! is visible on worker console
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/scripts/run_emscripten.sh | ||
|---|---|---|
| 104 ↗ | (On Diff #28819) | this is hiding file, comment to show |
| native/cpp/CommonCpp/DatabaseManagers/CommQueryExecutor.cpp | ||
|---|---|---|
| 25 ↗ | (On Diff #28819) | Could make this a bit more contextual. |
| web/cpp/Logger.cpp | ||
| 1 ↗ | (On Diff #28819) | Ideally ../../native/cpp/CommonCpp/ would be in the include path |
| web/scripts/run_emscripten.sh | ||
| 86 ↗ | (On Diff #28819) | |
| 104 ↗ | (On Diff #28819) | to delete the "backup file" |
Comment Actions
address review
| native/cpp/CommonCpp/DatabaseManagers/CommQueryExecutor.cpp | ||
|---|---|---|
| 25 ↗ | (On Diff #28819) | This class will be removed soon anyway, I wanted to show how I tested this, but I'll update this as you suggested |
| web/cpp/Logger.cpp | ||
| 1 ↗ | (On Diff #28819) | Good call - thanks. I will add Logger.h to avoid updating this at the end of the stack (because that's the way it is included in SQLiteQueryExecutor and #include "Tools/Logger.h" will cause compilation error) |
| web/scripts/run_emscripten.sh | ||
| 104 ↗ | (On Diff #28819) | if I recall correctly you requested to add this to .gitignore and not delete (for some reason), but okay and by "hiding file" I meant hiding this by Phabricator, it is caused because of the @generated phrase |
| web/scripts/run_emscripten.sh | ||
|---|---|---|
| 86 ↗ | (On Diff #28819) | explained in https://phab.comm.dev/D8548?id=28819#inline-55010 when I will do it, this line will cause compilation error in D8553 |
| web/cpp/Logger.cpp | ||
|---|---|---|
| 8 ↗ | (On Diff #28891) | Guess this could be a default implementation but also makes sense to be explicit. |
Comment Actions
@jon Could you please take a look at this and respond to my comment? The changes you requested are blocking the entire stack
| web/cpp/Logger.cpp | ||
|---|---|---|
| 8 ↗ | (On Diff #28891) | I prefer explicit separation instead of using default - seems more readable |