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
feat: add toggle arg to xdebug command, fixes #3593 #5625
feat: add toggle arg to xdebug command, fixes #3593 #5625
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file already has a mix of tabs and spaces for indentation so I'm really not sure what the indentation should be - and there's no guidance on that (at least that I could find) in https://ddev.readthedocs.io/en/stable/developers/building-contributing/#coding-style
Let me know if you want something specific to be done with the indents in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use spaces. You can add [skip ci]
to the commit message to avoid running all the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to make a commit here (will do a release soon, and want to merge it right now).
Download the artifacts for this pull request:
See Testing a PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GuySartorelli, thank you!
It looks like you've made all the necessary changes in all the necessary places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use spaces. You can add [skip ci]
to the commit message to avoid running all the tests.
Awesome, thank you for the quick turn around! 💙 |
The Issue
How This PR Solves The Issue
Adds new "toggle" argument to the
xdebug
commandManual Testing Instructions
ddev xdebug toggle
- see it enable xdebugddev xdebug toggle
- see it disable xdebugOptionally run
ddev xdebug status
inbetween to see that the status has actually changed.Automated Testing Overview
Added tests to validate that
ddev xdebug toggle
outputs the correct text to indicate the new status, and that runningddev xdebug status
afterwards reports that the status was in fact toggled.I'm not sure how to run tests locally so just wrote them blind and hoping they pass - but they're based on existing tests and it seems pretty straight forward so should be fine.
Related Issue Link(s)
#3593
Release/Deployment Notes
I don't think this has any other ramifications.
Other notes
I wanted to also change the default for
ddev xdebug
to be a shortcut forddev xdebug toggle
(it's currently a shortcut forddev xdebug on
) - but I figure that's probably a breaking change so if that does get done it should probably be done in a separate PR.