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

Move details bar to top in icicle mode. #177

Merged
merged 5 commits into from
Jun 3, 2020

Conversation

itamarst
Copy link
Contributor

@itamarst itamarst commented Jun 3, 2020

Fixes #176.

@itamarst itamarst changed the title WIP: Move details bar to top in icicle mode. Move details bar to top in icicle mode. Jun 3, 2020
@codecov
Copy link

codecov bot commented Jun 3, 2020

Codecov Report

Merging #177 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #177      +/-   ##
==========================================
- Coverage   90.23%   90.23%   -0.01%     
==========================================
  Files          16       16              
  Lines        2223     2232       +9     
==========================================
+ Hits         2006     2014       +8     
- Misses        217      218       +1     
Impacted Files Coverage Δ
src/flamegraph/mod.rs 96.40% <100.00%> (+0.09%) ⬆️
src/flamegraph/svg.rs 97.85% <100.00%> (+0.03%) ⬆️
src/collapse/common.rs 62.54% <0.00%> (-0.34%) ⬇️

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 aba81a3...4f0fedc. Read the comment docs.

@itamarst itamarst marked this pull request as ready for review June 3, 2020 20:21
@itamarst
Copy link
Contributor Author

itamarst commented Jun 3, 2020

Guess I should write a unit test too.

@itamarst
Copy link
Contributor Author

itamarst commented Jun 3, 2020

BTW, just realized you're in Cambridge, MA too. Could I get you to sign and share https://bit.ly/3eNdnhK? We're trying to hit 1000 people by the time we submit it tomorrow 😁

@jonhoo
Copy link
Owner

jonhoo commented Jun 3, 2020

I'm not currently in a position to test this, but I assume you have tested that both old and new flamegraphs still work? :p

@itamarst
Copy link
Contributor Author

itamarst commented Jun 3, 2020

Yeah. I can try to upload the results...

newcode.zip

@jonhoo
Copy link
Owner

jonhoo commented Jun 3, 2020

Looks great, thank you!

@jonhoo jonhoo merged commit b4b2757 into jonhoo:master Jun 3, 2020
jonhoo added a commit that referenced this pull request Jun 3, 2020
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.

In icicle mode, the bar at bottom should really be at the top
2 participants