Skip to content

Add endTime, mainMemory, arguments, symbolicationNotSupported to MetaInfo#4188

Merged
canova merged 13 commits into
firefox-devtools:mainfrom
parttimenerd:parttimenerd_meta_info_additions
Aug 18, 2022
Merged

Add endTime, mainMemory, arguments, symbolicationNotSupported to MetaInfo#4188
canova merged 13 commits into
firefox-devtools:mainfrom
parttimenerd:parttimenerd_meta_info_additions

Conversation

@parttimenerd

Copy link
Copy Markdown
Contributor

I want to add three different data points to the meta info (and show it in the MetaInfo box) which are especially helpful for imported profiles.

  • endTime: time the main process ended
  • mainMemory: size of the main memory (important for server applications)
  • arguments: program arguments
  • symbolicationNotSupported: if true, hides the symbolication related text and button, important for imported profiles that cannot be symbolicated (or are already symbolicated)

@parttimenerd parttimenerd requested a review from a team as a code owner August 17, 2022 12:20
@codecov

codecov Bot commented Aug 17, 2022

Copy link
Copy Markdown

Codecov Report

Merging #4188 (ff5fe30) into main (e809322) will increase coverage by 0.00%.
The diff coverage is 55.55%.

❗ Current head ff5fe30 differs from pull request most recent head e98060d. Consider uploading reports for the commit e98060d to get more accurate results

@@           Coverage Diff           @@
##             main    #4188   +/-   ##
=======================================
  Coverage   88.44%   88.44%           
=======================================
  Files         280      280           
  Lines       24552    24536   -16     
  Branches     6548     6543    -5     
=======================================
- Hits        21714    21702   -12     
+ Misses       2636     2632    -4     
  Partials      202      202           
Impacted Files Coverage Δ
src/components/app/MenuButtons/MetaInfo.js 82.71% <55.55%> (-3.40%) ⬇️
src/selectors/per-thread/index.js 96.15% <0.00%> (ø)
src/components/stack-chart/index.js 97.77% <0.00%> (ø)
src/components/flame-graph/Canvas.js 92.06% <0.00%> (ø)
src/components/stack-chart/Canvas.js 94.44% <0.00%> (ø)
src/components/flame-graph/FlameGraph.js 78.88% <0.00%> (ø)
src/profile-logic/profile-data.js 90.54% <0.00%> (+0.14%) ⬆️
src/components/shared/CallNodeContextMenu.js 90.00% <0.00%> (+0.32%) ⬆️
src/selectors/profile.js 97.00% <0.00%> (+0.80%) ⬆️
src/components/tooltip/CallNode.js 88.09% <0.00%> (+0.88%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@flodolo flodolo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please don't touch localized files, only add strings to en-US

Comment thread locales/en-US/app.ftl Outdated
Comment thread locales/en-US/app.ftl Outdated
Comment thread src/types/profile.js Outdated
@parttimenerd

parttimenerd commented Aug 17, 2022

Copy link
Copy Markdown
Contributor Author

A sample file: profile.json.zip

Comment thread locales/en-US/app.ftl Outdated

@canova canova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR! Looks pretty good to me. I've added some comments, we can land it after they are addressed. Let me know if you have any question.

Comment thread src/components/app/MenuButtons/MetaInfo.js Outdated
Comment thread locales/en-US/app.ftl
Comment thread src/components/app/MenuButtons/MetaInfo.js Outdated
Comment thread src/types/profile.js Outdated
Comment thread src/components/app/WindowTitle.js Outdated
Comment thread src/components/app/MenuButtons/MetaInfo.js Outdated
Comment thread src/components/app/WindowTitle.js Outdated
@parttimenerd

Copy link
Copy Markdown
Contributor Author

Is there anything for me to do, before it can be merged?

@mstange

mstange commented Aug 18, 2022

Copy link
Copy Markdown
Contributor

I'd like @canova to take one last look, because he has commented before and not approved yet.

@canova canova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me too, thanks!

@canova canova enabled auto-merge August 18, 2022 13:28
@canova canova merged commit 55284c1 into firefox-devtools:main Aug 18, 2022
@parttimenerd parttimenerd deleted the parttimenerd_meta_info_additions branch August 18, 2022 13:42
@canova canova mentioned this pull request Sep 6, 2022
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.

4 participants