sentry-cli icon indicating copy to clipboard operation
sentry-cli copied to clipboard

feat(profiling): add profiling to debug_files commands

Open viglia opened this issue 3 years ago • 4 comments

viglia avatar Sep 13 '22 14:09 viglia

It looks like you rebased something on top of latest master branch and my recent commit is showing up here. Can you fix that please?

kamilogorek avatar Sep 13 '22 14:09 kamilogorek

Fixed linting issue.

kamilogorek avatar Sep 22 '22 14:09 kamilogorek

I’m also not sure if some of these commands panic instead of returning an error, @kamilogorek can enlighten us here.

Yes, it can panic, and we have a hook for that in place as well https://github.com/getsentry/sentry-cli/blob/0c4c6de12c26663fcce94422c8ab8113a9779714/src/utils/system.rs#L140-L170

kamilogorek avatar Sep 27 '22 09:09 kamilogorek

I'd revert all the changes from Wraps command in a closure to make sure the transaction ends regardless of the results commit tbh, it's very noisy, and changes this PRs diff by hundreds of lines.

Cleaner solution IMO (at least for now), would be to modify debug_files/mod.rs, where we actually register and execute those commands. This way commands are self-contained, and profiling is only added as sort of "instrumentation" of those commands.

kamilogorek avatar Sep 27 '22 09:09 kamilogorek

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions[bot] avatar Oct 19 '22 00:10 github-actions[bot]

@viglia looks like this PR has been open for some time. Are the changes still relevant, and if yes, is the PR ready for review?

szokeasaurusrex avatar Jan 31 '24 10:01 szokeasaurusrex

@szokeasaurusrex, I'm closing it since it's not relevant anymore.

viglia avatar Jan 31 '24 14:01 viglia