-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom Label Logic #411
Custom Label Logic #411
Conversation
lib/kino/process.ex
Outdated
@@ -224,6 +225,14 @@ defmodule Kino.Process do | |||
target argument can either be a single PID, a list of PIDs, or the atom `:all` | |||
depending on what messages you would like to retain in your trace. | |||
|
|||
## Options | |||
|
|||
* `:label_function` - A function to help label message events. If |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call it :message_label
? We usually don't append the type to the option names.
I also don't think we should expose label_from_value
. If you pass a custom function, you usually know better which values you are going to handle, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call it :message_label? We usually don't append the type to the option names.
Sure, let me change that now.
I also don't think we should expose label_from_value. If you pass a custom function, you usually know better which values you are going to handle, no?
Most likely, I just wanted to preserve the initial message, as the code I'll be running it on just wraps the normal GenServer.call
and GenServer.cast
with some extra information to the given GenServer
, meaning the default behavior is close to what I wanted but not quite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both complaints should be addressed now on the latest push
Namely we add message labeling to the code, so that a user can override the default labeling logic
We add documentation on how this could be used
66f6671
to
09a2ca7
Compare
I have also provided an example in the documentation now on how to run the new |
💚 💙 💜 💛 ❤️ |
This topic addresses #408
I added an extra argument to
seq_trace
andrender_seq_trace
, that accepts an extra argument to augment the labeling logic.I have not tested the code yet, nor did I see any tests in the repo.
I hope it works.
I am now going to test this in a project I'm working on, and might add an extra commit that augments the example, showing off what the custom label can do.
I've also exposed
label_from_value
as I think it may add some value to people's custom's logic'sis one such logic I used in mocking up what I wanted to do. I will get a better example as I make more examples.