Skip to content
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

Make behavior of skip_after more consistent. #224

Merged
merged 2 commits into from
Dec 19, 2021
Merged

Conversation

romange
Copy link
Contributor

@romange romange commented Dec 6, 2021

Specifically, omits process name as a root frame if skip_after condition was trigerred.
Also, I added "skip-after" flag to collapse-perf.rs in order to actually be able to use
and test this flag on real stack traces.

@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #224 (7061145) into master (a29c8d8) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 7061145 differs from pull request most recent head 3af530f. Consider uploading reports for the commit 3af530f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #224      +/-   ##
==========================================
+ Coverage   87.00%   87.03%   +0.02%     
==========================================
  Files          17       17              
  Lines        2378     2383       +5     
==========================================
+ Hits         2069     2074       +5     
  Misses        309      309              
Impacted Files Coverage Δ
src/collapse/perf.rs 97.85% <100.00%> (+0.02%) ⬆️
src/collapse/dtrace.rs 90.67% <0.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a29c8d8...3af530f. Read the comment docs.

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Just two minor nits. Thanks!

src/bin/collapse-perf.rs Show resolved Hide resolved
src/collapse/perf.rs Outdated Show resolved Hide resolved
Specifically, omits process name as a root frame if `skip_after` condition was trigerred.
Also, I added "skip-after" flag to collapse-perf.rs in order to actually be able to use
and test this flag on real stack traces.
@romange
Copy link
Contributor Author

romange commented Dec 14, 2021

@jonhoo addressed your comments. Please advise if the comment above is not clear.

src/bin/collapse-perf.rs Outdated Show resolved Hide resolved
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Thanks!

@jonhoo jonhoo merged commit bb737d0 into jonhoo:master Dec 19, 2021
@jonhoo
Copy link
Owner

jonhoo commented Dec 19, 2021

Released in 0.10.9 🎉

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