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

Reword VarDumper description #31339

Merged
merged 1 commit into from May 1, 2019

Conversation

greg0ire
Copy link
Contributor

@greg0ire greg0ire commented May 1, 2019

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

It is hard to understand what "Built on top" refers to, and even when
knowing, the sentence looks weird.
Untitled

The description of the Github repository has the same issue and should be changed.

@@ -2,8 +2,8 @@ VarDumper Component
===================

The VarDumper component provides mechanisms for walking through any arbitrary
PHP variable. Built on top, it provides a better `dump()` function that you
can use instead of `var_dump`.
PHP variable. It also provides a better `dump()` function that you can use
Copy link
Member

Choose a reason for hiding this comment

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

I think it would work better without "also".

@nicolas-grekas
Copy link
Member

Linking to Slack is like linking to an oral discussion: it will disappear from memories quite soon and won't allow others to get the arguments soon too.
For 3.4?

@greg0ire greg0ire force-pushed the var-dumper-description-wording branch from 3413560 to 6c44f08 Compare May 1, 2019 09:15
@greg0ire greg0ire changed the base branch from master to 3.4 May 1, 2019 09:15
@greg0ire
Copy link
Contributor Author

greg0ire commented May 1, 2019

For 3.4?

I changed the base. I assumed pedantic changes that are not bugfixes should go on master :P

@greg0ire
Copy link
Contributor Author

greg0ire commented May 1, 2019

Linking to Slack is like linking to an oral discussion: it will disappear from memories quite soon and won't allow others to get the arguments soon too.

It was provided as a bonus, I replaced it with an anonymized screenshot.

It is hard to understand what "Built on top" refers to, and even when
knowing, the sentence looks weird.
@greg0ire greg0ire force-pushed the var-dumper-description-wording branch from 6c44f08 to 6024e16 Compare May 1, 2019 09:52
@chalasr chalasr added this to the 3.4 milestone May 1, 2019
@fabpot
Copy link
Member

fabpot commented May 1, 2019

Thank you @greg0ire.

@fabpot fabpot merged commit 6024e16 into symfony:3.4 May 1, 2019
fabpot added a commit that referenced this pull request May 1, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Reword VarDumper description

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

It is hard to understand what "Built on top" refers to, and even when
knowing, the sentence looks weird.
![Untitled](https://user-images.githubusercontent.com/657779/57011659-15ebfe00-6c03-11e9-9b85-5c00cfc15026.png)

The description of [the Github repository](https://github.com/symfony/var-dumper) has the same issue and should be changed.

Commits
-------

6024e16 Reword VarDumper description
@greg0ire greg0ire deleted the var-dumper-description-wording branch May 1, 2019 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants