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
- Branch
- wasm-publish
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/scripts/run_emscripten.sh | ||
|---|---|---|
| 104 | this is hiding file, comment to show | |
Comment Actions
address review
| native/cpp/CommonCpp/DatabaseManagers/CommQueryExecutor.cpp | ||
|---|---|---|
| 25 | 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 | 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 | 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 | 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 |