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

A few improvements for the README.md #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rpkoller
Copy link

An initial rough draft for the README.md (based on my comment in #23). The following things I would suggest to change.

  • A few times the wordmark XHGui was only written in lower case.
  • Each "Framework configuration"-section had the following redundant snippet:
Run ddev xhprof to start profiling.
XHGui is now available at https://yourproject.ddev.site:8142

Starting the profiling is only relevant to the usage section imho. therefore i've moved the how to enable xhprof to the usage section. plus there you also already have a link with the port as well.

  • In the usage section i've removed The service will automatically start when run: ddev start or ddev restart. the project will start anyway even if it was stopped by just running ddev xhprof on or ddev xhprof on && ddev xhgui

@rpkoller rpkoller changed the title 20231129 rpkoller readme improvements A few improvements for the README.md Nov 29, 2023
@tyler36
Copy link
Collaborator

tyler36 commented Nov 29, 2023

Thank you for the PR.

Good call on the capitalization issues.

I deliberately repeated ddev xhprof steps in each framework section to get it to a "working state"; install addon, follow framework guide. But I can see your point on centralizing into a usage section.

Any thoughs @rfay? I'll merge if no-one has any tweaks.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I suspect that @rpkoller didn't understand that you do ddev xhprof on to start profiling and you do ddev xhgui to study the results. So they may not be done at the same time. People are going to have to understand the difference.

In the long run, I think the xhprof custom command should have a start and a stop in it perhaps, that wraps the "turn on profiling" and the "turn off profiling" into the same command.

I'm not entirely sure from a quick read through that this is going the right direction, but I do think it needs to be smoothed out, but probably not just with docs.

I think we should probably rework the approach to xhprof in general (in core) so that some of the sticky things we're dealing with here can be smoothed out.

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.

3 participants