Skip to content

[PropertyInfo] Auto-enable PropertyInfo component #27429

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

Merged
merged 1 commit into from
May 31, 2018

Conversation

sroze
Copy link
Contributor

@sroze sroze commented May 30, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ø
License MIT
Doc PR ø

The PropertyInfo component (used by the Serializer to provide more insights from PhpDocs, etc...) is disabled by default, not sure why. This enables it by default when the component is installed.

@sroze
Copy link
Contributor Author

sroze commented May 30, 2018

To be fair, this could even be considered as a bug and added from 3.4. WDYT?

@sroze sroze changed the title [PropertyInfo] Auto-enabled PropertyInfo component [PropertyInfo] Auto-enable PropertyInfo component May 30, 2018
@sroze
Copy link
Contributor Author

sroze commented May 30, 2018

There is a test that ensures that property info is disabled by default. Did I miss something or should I remove it?

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

It has been forgotten when migrating from the fullstack framework to Flex. It's also a bug fix for me.

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.

Should be done on master (not a bug).

@sroze
Copy link
Contributor Author

sroze commented May 30, 2018

(removed the test that forces the property info to be disabled)

@sroze
Copy link
Contributor Author

sroze commented May 30, 2018

AppVeyor's failure is unrelated.

Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Good catch

@fabpot fabpot force-pushed the auto-enable-property-info branch from e9e36a5 to 06ea72e Compare May 31, 2018 05:58
@fabpot
Copy link
Member

fabpot commented May 31, 2018

Thank you @sroze.

@fabpot fabpot merged commit 06ea72e into symfony:master May 31, 2018
fabpot added a commit that referenced this pull request May 31, 2018
This PR was squashed before being merged into the 4.2-dev branch (closes #27429).

Discussion
----------

[PropertyInfo] Auto-enable PropertyInfo component

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ø
| License       | MIT
| Doc PR        | ø

The PropertyInfo component (used by the Serializer to provide more insights from PhpDocs, etc...) is disabled by default, not sure why. This enables it by default when the component is installed.

Commits
-------

06ea72e [PropertyInfo] Auto-enable PropertyInfo component
This was referenced Nov 3, 2018
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.

5 participants