Conversation
goto-bus-stop
left a comment
There was a problem hiding this comment.
two nits. we should also add a test but we can discuss that in slack a bit later.
|
(This functionality is tested in clinicjs/node-clinic-flame#107. Flame is a bit better set up for testing atm 😬 ) @DylanC we need a line of documentation for this option in |
|
@goto-bus-stop - I have both of those updates pushed into the branch. |
|
@goto-bus-stop - Should I wait until we figure out the tests for Flame first? Seems testing this would have the same issue? |
|
Wondering if there's anyone around to help me revive this PR? I have a rebased version of this running here with added usage info + a minor tweak: https://github.com/TryGhost/0x It is working & useful - it sort of solves #129 as although you can't explicitly say when to start profiling, you can skip the startup process of a script and only measure e.g. when firing requests. I did look at adding the requested unit tests, but as far as I can tell the code that this affects in v8-log-to-ticks is currently untested, meaning adding a test for this behaviour is a fairly big task, and I don't really know where to start. I don't really want to run a fork, ideally I'd like to get this populated through flame & clinic as was the intention of the original author. |
|
would you like to do a fresh PR? |
|
Done: #239 :) |
|
Heads up there's nothing in here that wasn't in #239, and there for this can be closed 😺 |
Putting this up early to have eyes on it. Node Clinic will have a new feature to set a timed delay before we collect the data.
Relevant PR:
clinicjs/node-clinic#208
For 0x the delay won't hold up the process but rather filter the data depending on the delay value passed in. Open to feedback and further changes/suggestions.