Skip to content

Conversation

@Mitchell-Ryan
Copy link
Contributor

No description provided.

Copy link
Member

@dirkdev98 dirkdev98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion!

Would prefer to prevent words like 'powerful', 'valuable', 'easily', etc. It all depends on the context, what the user is used to, what tools they have to analyse the logs, etc.

I'd expect How to use the Insight Event: to give a few more examples based on what functions are called, ie:

/**
 * How to use the Insight Event:
 *
 * Start by retrieving a root event. It can be created by calling {@link newEvent} and passing it a logger. When you use the {@link getApp} from @compas/store, it automatically adds a root event to `ctx.event`. In your tests you can use {@link newTestEvent}.
 * ...
 * Finally, when the root event is stopped via {@link eventStop}, the event logs the stat and end times of the function execution. ...

And a bit nitty, but I'd prefer to use less uppercased words, ie Logging the Duration would be Logging the duration

@Mitchell-Ryan
Copy link
Contributor Author

Mitchell-Ryan commented Jul 15, 2023

Very valid points, and documentation can never be too nitty (A).

Will make a new suggestion.

@Mitchell-Ryan Mitchell-Ryan requested a review from dirkdev98 July 28, 2023 13:47
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #2728 (5f0568c) into main (eaa0ad2) will increase coverage by 0.01%.
Report is 30 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2728      +/-   ##
==========================================
+ Coverage   67.13%   67.14%   +0.01%     
==========================================
  Files         217      217              
  Lines       39426    39439      +13     
==========================================
+ Hits        26467    26480      +13     
  Misses      12959    12959              

Copy link
Member

@dirkdev98 dirkdev98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, thanks!

@dirkdev98 dirkdev98 merged commit 9a8afc4 into compasjs:main Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants