Skip to content

[FrameworkBundle] Finetune AboutCommand #57829

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 7, 2024
Merged

[FrameworkBundle] Finetune AboutCommand #57829

merged 1 commit into from
Oct 7, 2024

Conversation

JoppeDC
Copy link
Contributor

@JoppeDC JoppeDC commented Jul 25, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
License MIT
  • Make the AboutCommand show more details about the XDebug extension (Installed vs Enabled).
  • Show the same info about the OPcache and APCu extensions

@carsonbot carsonbot added this to the 7.2 milestone Jul 25, 2024
javiereguiluz
javiereguiluz approved these changes Jul 25, 2024
View reviewed changes
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

Good idea Joppe!

Just displaying true/false is not clear enough to distinct between installed or not and enabled or not. Thanks!

ro0NL
ro0NL reviewed Jul 25, 2024
View reviewed changes
ro0NL

This comment was marked as outdated.

Sign in to view

OskarStark
OskarStark reviewed Jul 25, 2024
View reviewed changes
ro0NL
ro0NL suggested changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

the same is in webprofiler, it should be in sync

@carsonbot carsonbot changed the title Finetune AboutCommand [FrameworkBundle] Finetune AboutCommand Jul 28, 2024
@OskarStark OskarStark changed the title [FrameworkBundle] Finetune AboutCommand [FrameworkBundle] Finetune AboutCommand Jul 28, 2024
@OskarStark
Copy link
Contributor

the same is in webprofiler, it should be in sync

After this comment 👍

@OskarStark
Copy link
Contributor

@JoppeDC open for finishing this PR? Thanks

@JoppeDC
Copy link
Contributor Author

JoppeDC commented Aug 2, 2024

@JoppeDC open for finishing this PR? Thanks

Absolutely. I'll have a look at the profiler changes

nicolas-grekas
nicolas-grekas reviewed Aug 13, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Status: needs work

@javiereguiluz javiereguiluz added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 3, 2024
@javiereguiluz
Copy link
Member

@JoppeDC if you have some time, we need some last effort from you to fix some minor issues here. Then, we'll merge this in time for Symfony 7.2 release. Thanks!

@JoppeDC
Copy link
Contributor Author

JoppeDC commented Oct 3, 2024

@javiereguiluz I've quickly fixed the feedback issues that were still open.
I have not had the time (and also i totally forgot) to check out the implementation in the Profiler tho

GromNaN
GromNaN approved these changes Oct 7, 2024
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

LGTM

fabpot
fabpot approved these changes Oct 7, 2024
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Small CS fixes needed.

@fabpot
Copy link
Member

fabpot commented Oct 7, 2024

Thank you @JoppeDC.

@fabpot fabpot merged commit 6864dbe into symfony:7.2 Oct 7, 2024
9 of 10 checks passed
@fabpot fabpot mentioned this pull request Oct 7, 2024
Closed
@fabpot fabpot mentioned this pull request Oct 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@javiereguiluz javiereguiluz javiereguiluz approved these changes

@GromNaN GromNaN GromNaN approved these changes

@OskarStark OskarStark OskarStark approved these changes

@xabbuh xabbuh xabbuh approved these changes

+1 more reviewer

@ro0NL ro0NL ro0NL requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Feature FrameworkBundle ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
9 participants
@JoppeDC @OskarStark @javiereguiluz @fabpot @nicolas-grekas @GromNaN @ro0NL @xabbuh @carsonbot