Skip to content

[FrameworkBundle] Add default mapping path for serializer component in bundle-less app #24833

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
Nov 10, 2017
Merged

[FrameworkBundle] Add default mapping path for serializer component in bundle-less app #24833

merged 1 commit into from
Nov 10, 2017

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Nov 6, 2017 •
edited
Loading

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

http://symfony.com/doc/current/serializer.html#using-serialization-groups-annotations:
In addition to the @groups annotation, the Serializer component also supports Yaml or XML files. These files are automatically loaded when being stored in one of the following locations:

  • The serialization.yml or serialization.xml file in the Resources/config/ directory of a bundle;
  • All *.yml and *.xml files in the Resources/config/serialization/ directory of a bundle.

Inspired by the second convention, this proposal adds one more but for bundle-less structure. Theoretically this is what it does for you:

framework:
    serializer:
        mapping:
            paths:
                - '%kernel.project_dir%/config/serializer/'

@yceruto yceruto changed the title [FrameworkBundle] Add groups configuration for serializer component in bundle-less application [FrameworkBundle] Add default mapping path for serializer component in bundle-less app Nov 6, 2017
@yceruto
Copy link
Member Author

yceruto commented Nov 6, 2017 •
edited
Loading

I just see this issue #21187 related to the same proposal by @chalasr:

Shouldn't we try to detect those configuration files in %kernel.root_dir%/Resources/config/serialization as well?

I hope it makes more sense now for Symfony 4 structure.

@yceruto yceruto force-pushed the serializer_path branch 5 times, most recently from d4b9c2a to a1d852d Compare November 6, 2017 05:39
@ogizanagi
Copy link
Contributor

ogizanagi commented Nov 6, 2017 •
edited
Loading

Not sure the Resources directory still makes sense for this with Flex. Ideally, if Symfony has adopted the root res/ directory, I'd put it there in a res/serializer directory, next to templates, translations, validation mapping & other resources used by the application.
But for now, I put it in config/serializer. Not src/Resources/config/serializer. Actually my kernel isn't even at src/ root. IMHO %kernel.root_dir% should be avoided as much as possible now.

@sroze
Copy link
Contributor

sroze commented Nov 6, 2017

AFAIK res/ did not happen yet so I'd go for config/serializer, that would make sense to me.

@yceruto
Copy link
Member Author

yceruto commented Nov 6, 2017

Changed to %kernel.project_dir%/config/serializer/ by now 👍

@xabbuh xabbuh added this to the 4.1 milestone Nov 6, 2017
dunglas
dunglas approved these changes Nov 6, 2017
View reviewed changes
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.

Should be merged in 3.4 as a bug fix. We should probably do the same for the validator component.

@yceruto yceruto mentioned this pull request Nov 7, 2017
Merged
@yceruto
Copy link
Member Author

yceruto commented Nov 7, 2017

@dunglas great! see #24856 for the Validator component.

@yceruto yceruto changed the base branch from master to 3.4 November 7, 2017 13:57
@yceruto
Copy link
Member Author

yceruto commented Nov 7, 2017

Rebased to 3.4 (same opinion of @nicolas-grekas here #24856 (comment))

@chalasr
Copy link
Member

chalasr commented Nov 9, 2017

So, we are moving from config/serialization/ to config/serializer/ (same for validation/validator in #24856)?
No strong opinion on that, slightly prefer serializer as it reflects the package name.
Shouldn't the new name be supported for other paths (e.g. Resources/config/serializer for bundles)?

@fabpot
Copy link
Member

fabpot commented Nov 10, 2017

I don't think we need to support the new name for the old paths.

@fabpot
Copy link
Member

fabpot commented Nov 10, 2017

Thank you @yceruto.

@fabpot fabpot merged commit 43895b8 into symfony:3.4 Nov 10, 2017
fabpot added a commit that referenced this pull request Nov 10, 2017
@fabpot
bug #24833 [FrameworkBundle] Add default mapping path for serializer …
93f206e 
…component in bundle-less app (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Add default mapping path for serializer component in bundle-less app

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

> http://symfony.com/doc/current/serializer.html#using-serialization-groups-annotations:
> In addition to the @groups annotation, the Serializer component also supports Yaml or XML files. These files are automatically loaded when being stored in one of the following locations:
>* The `serialization.yml` or `serialization.xml` file in the `Resources/config/` directory of a bundle;
>* All `*.yml` and `*.xml` files in the `Resources/config/serialization/` directory of a bundle.

Inspired by the second convention, this proposal adds one more but for bundle-less structure. Theoretically this is what it does for you:
```yaml
framework:
    serializer:
        mapping:
            paths:
                - '%kernel.project_dir%/config/serializer/'
```

Commits
-------

43895b8 Add default mapping path for serializer component
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Nov 10, 2017
@fabpot
bug #24856 [FrameworkBundle] Add default mapping path for validator c…
9002559 
…omponent in bundle-less app (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Add default mapping path for validator component in bundle-less app

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

Same approach of symfony/symfony#24833

Commits
-------

4e0daecc63 Add default mapping path for validator component
fabpot added a commit that referenced this pull request Nov 10, 2017
@fabpot
bug #24856 [FrameworkBundle] Add default mapping path for validator c…
c549e3c 
…omponent in bundle-less app (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Add default mapping path for validator component in bundle-less app

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

Same approach of #24833

Commits
-------

4e0daec Add default mapping path for validator component
@yceruto yceruto deleted the serializer_path branch November 10, 2017 19:20
This was referenced Nov 12, 2017
Merged
Merged
@yceruto yceruto mentioned this pull request Nov 13, 2017
Closed
@yceruto yceruto mentioned this pull request Nov 21, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@dunglas dunglas dunglas approved these changes

Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
8 participants
@yceruto @ogizanagi @sroze @chalasr @fabpot @dunglas @xabbuh @carsonbot