-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add collector custom arguments #85
base: master
Are you sure you want to change the base?
Conversation
9f2b024
to
804fc90
Compare
804fc90
to
1c72671
Compare
In that way, we can: - interact with collector hooks at runtime - modulate the item properties based on user input
1c72671
to
e23599d
Compare
This shows that we can use external data inside collector hooks
Thanks so much for sending this our way, @aoblet. I've logged an internal ticket to have it reviewed, so we will take a closer look as soon as we can. |
Hi @aoblet , I'm sorry we haven't been able to get back to your PR yet. I think I may have given the impression when we were discussing on Zendesk that we'd look at this right away, but it's become clear to us that we're kinda slammed at the moment and are not able to take the time we need to review client PRs and take them through the finish line. I'm deeply sorry about this. Have you had success using these changes in production? Is there anything you feel like you'd like to improve? |
No worries. On our side, these changes are in production and are doing the job. |
@aoblet @jfboismenu Hi guys, any update about this PR ? |
Hi @eprana - could you tell us a little more about how you've been using this functionality in production? I'm curious about the specific use cases that you've been able to address with the collector custom arguments that weren't possible before. |
This allows to:
By refactoring the way we prepare arguments to
run_process_file
andrun_process_file
, maybe that_get_process_args
is less easier to read than before.. What do you think ?