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

Include note of common profiling issue #9484

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

tbillington
Copy link
Contributor

  • Includes a note about a common issue seen when profiling with span based tracing tools
  • Adds a table of contents to profile.md

Rendered.

Suggested by Jasmine on discord.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

This is a nice improvement! Both the common issue and the cleanup are nice.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Rendering Drawing game state to the screen labels Aug 19, 2023
Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

Thanks for writing this! Some minor wording changes and then we're good to go.

docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
@tbillington tbillington requested a review from JMS55 August 20, 2023 02:42
@JMS55 JMS55 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 20, 2023
Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

You need to increase all your heading levels by 1, there can be only one #-level heading.

The linter is also unhappy about your note: https://github.com/updownpress/markdown-lint/blob/master/rules/036-no-emphasis-as-header.md

docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
@mockersf
Copy link
Member

it seems it's the first time the linter passed on this file, and it's unhappy with the list made with *, could you replace it with -?

@tbillington
Copy link
Contributor Author

I was following the patterns in https://raw.githubusercontent.com/bevyengine/bevy/v0.11.2/examples/README.md.

Will update them in the morning, didn't know about the markdown linter, thanks!

docs/profiling.md Outdated Show resolved Hide resolved
@tbillington tbillington requested a review from mockersf August 21, 2023 00:39
@mockersf mockersf added this pull request to the merge queue Aug 22, 2023
Merged via the queue into bevyengine:main with commit ebdf506 Aug 22, 2023
@tbillington tbillington deleted the profiling-doc-update branch August 22, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants