Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
junhaoliao created a review on a pull request on y-scope/clp-ffi-js
compared with https://github.com/y-scope/yscope-log-viewer/pull/121 and this lgtm
kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
Can we move those into StreamReader? I think the only difference (besides the template parameter) is that one is a shared_ptr and one isn't, right?
coderabbitai[bot] created a review comment on a pull request on y-scope/clp-ffi-js
_:warning: Potential issue_ **Ensure `StructuredIrUnitHandler` inherits from `IrUnitHandlerInterface`.** The class documentation indicates that `StructuredIrUnitHandler` implements `clp::ffi::ir_...
coderabbitai[bot] created a review comment on a pull request on y-scope/clp-ffi-js
_:warning: Potential issue_ **Add overflow check for node ID increment** The current node ID increment could potentially overflow. Consider adding bounds checking. ```diff + if (m_current_nod...
coderabbitai[bot] created a review comment on a pull request on y-scope/clp-ffi-js
_:warning: Potential issue_ **Add bounds checking for timestamp casting** The current implementation might overflow when casting large integers to epoch_time_ms_t. ```diff + auto const ra...
coderabbitai[bot] created a review comment on a pull request on y-scope/clp-ffi-js
_:hammer_and_wrench: Refactor suggestion_ **Add input validation and improve error handling** Consider validating the input log event and handling potential errors from get_timestamp and get_log_...
coderabbitai[bot] created a review on a pull request on y-scope/clp-ffi-js
**Actionable comments posted: 4** <details> <summary>🧹 Outside diff range and nitpick comments (4)</summary><blockquote> <details> <summary>src/clp_ffi_js/ir/StructuredIrUnitHandler.cpp (3)</summ...
kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
I haven't tried this, so maybe there's some unforeseen limitation, but I think a clean approach is: * Make StreamReader a template class that takes the log event type as a template parameter. *...
LinZhihao-723 created a review comment on a pull request on y-scope/clp-ffi-js
- We don't need to check any error code on the end of stream; the end of stream is handled as an IR unit; you can check the returned IR unit type to confirm (but that should also be a part of the j...
kirkrodrigues created a comment on a pull request on y-scope/clp-ffi-js
Sorry, can we update it to follow https://github.com/y-scope/clp-loglib-py/blob/main/.github/workflows/pr-title-checks.yaml? That's the latest version of this that we should propagate across all re...
junhaoliao created a review on a pull request on y-scope/clp-ffi-js
compared with https://github.com/y-scope/yscope-log-viewer/pull/121 and this lgtm
coderabbitai[bot] created a review on a pull request on y-scope/clp-ffi-js
**Actionable comments posted: 0** <details> <summary>🧹 Outside diff range and nitpick comments (1)</summary><blockquote> <details> <summary>.github/workflows/pr-title-checks.yaml (1)</summary><bl...
coderabbitai[bot] created a comment on a pull request on y-scope/clp-ffi-js
<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- This is an auto-generated comment: review in progress by coderabbit.ai --> > [!NOTE] > Currently processing new changes ...
coderabbitai[bot] created a review on a pull request on y-scope/clp-ffi-js
**Actionable comments posted: 0** <details> <summary>🧹 Outside diff range and nitpick comments (2)</summary><blockquote> <details> <summary>src/clp_ffi_js/ir/StructuredIrUnitHandler.hpp (2)</summ...
coderabbitai[bot] created a review on a pull request on y-scope/clp-ffi-js
**Actionable comments posted: 0** <details> <summary>🧹 Outside diff range and nitpick comments (7)</summary><blockquote> <details> <summary>src/clp_ffi_js/ir/utils.hpp (4)</summary><blockquote> ...
kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
1. Sure 2. clang-tidy [discourages](https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.html) variadic functions. 3. Sure, but how would that help?
junhaoliao created a review comment on a pull request on y-scope/clp-ffi-js
Can we lift this variable and also the length reservation outside of the for-loop?
coderabbitai[bot] created a review comment on a pull request on y-scope/clp-ffi-js
@junhaoliao, you're correct. The expression `log_event_idx + 1` does not pose an integer overflow risk since `log_event_idx` is of type `size_t`. My previous suggestion was unnecessary. I apologize...
junhaoliao created a review comment on a pull request on y-scope/clp-ffi-js
i'm not sure how your suggestion would help. could you explain a bit?
junhaoliao created a review comment on a pull request on y-scope/clp-ffi-js
more of a preference. feel free to disagree ```suggestion auto const& timestamp_pair{id_value_pairs.at(m_timestamp_node_id.value())}; if (false == timestamp_pair.has_value()) { ...
junhaoliao created a review comment on a pull request on y-scope/clp-ffi-js
@kirkrodrigues 1. Shall we use `std::enable_if<std::is_same_v<>>` for `ts_pattern`? 2. if not, do we consider Pack / variadic arguments a bad coding practice in C++? can that be used here? 3. i...
junhaoliao created a review comment on a pull request on y-scope/clp-ffi-js
How about ```suggestion * Buffers the log event with metadata extracted. ```