Skip to content

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented Jun 7, 2016

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

Improve api file configuration resources, see #421 discussion

This changes the default filename for resources configuration from resources.{yml,xml} to api_resources.{yml,xml,yaml}. It also enables per-file configurations that would have to be located in Resources/config/api_resources/*.{yml,xml,yaml}.

Note that I changed the fixture paths for my dummy resources to be less messy.

@soyuka soyuka changed the title Change configuration filename to api_resources.{xml|yml} and add single resou Configuration filename to api_resources and allow single file declaration Jun 7, 2016
{
$paths = [];
$globOptions = GLOB_BRACE | GLOB_NOSORT;
$prefix = DIRECTORY_SEPARATOR.'Resources'.DIRECTORY_SEPARATOR.'config'.DIRECTORY_SEPARATOR;
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about using the Symfony Finder Component instead of glob directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can but I wanted to avoid adding a dependency only for this.
Le mar. 7 juin 2016 à 12:07, Kévin Dunglas notifications@github.com a
écrit :

In src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
#568 (comment):

@@ -149,12 +149,16 @@ private function registerAnnotationLoaders(ContainerBuilder $container)
private function registerFileLoaders(ContainerBuilder $container)
{
$paths = [];

  •    $globOptions = GLOB_BRACE | GLOB_NOSORT;
    
  •    $prefix = DIRECTORY_SEPARATOR.'Resources'.DIRECTORY_SEPARATOR.'config'.DIRECTORY_SEPARATOR;
    

What do you think about using the Symfony Finder Component instead of glob
directly?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/api-platform/core/pull/568/files/fd99357da41448ed77aceac668050c0b1b7d3ad8#r66043028,
or mute the thread
https://github.com/notifications/unsubscribe/ABQr81YXxO09RMmxY-MAwF1LJ_-k7Z6Dks5qJULFgaJpZM4Ivt1f
.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough.

@dunglas
Copy link
Member

dunglas commented Jun 7, 2016

👍 ping @api-platform/core-team

$paths = [];
$globOptions = GLOB_BRACE | GLOB_NOSORT;
$prefix = DIRECTORY_SEPARATOR.'Resources'.DIRECTORY_SEPARATOR.'config'.DIRECTORY_SEPARATOR;
$apiResourcesGlob = $prefix.'api_resources.{xml,yml}';
Copy link
Contributor

Choose a reason for hiding this comment

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

you should also allow .yaml, it's not very common in Sf but a perfectly valid extension nonetheless.

@soyuka soyuka force-pushed the changeconfigname branch from fd99357 to f8d2dd7 Compare June 7, 2016 16:06
@dunglas
Copy link
Member

dunglas commented Jun 7, 2016

I'll release an alpha 2 before the end of this week. It would have this PR merged. @soyuka wdyt about using the SF finder to make @theofidry happy? :)

@soyuka
Copy link
Member Author

soyuka commented Jun 7, 2016

From what I understood he want me to move the "finder-like" code to a new class named "Finder" on api-platform. If implementing symfony's Finder makes everyone happy for sure!

Btw sorry but I won't have time to add configurable properties until the end of this week (not so a BC break so it doesn't matter I hope)!

@dunglas
Copy link
Member

dunglas commented Jun 7, 2016

@soyuka right, it's not a BC break so it's perfectly ok to add it later. Thanks for all your work.

@soyuka soyuka force-pushed the changeconfigname branch from 7f52869 to f8da3dc Compare June 7, 2016 18:54
@soyuka
Copy link
Member Author

soyuka commented Jun 7, 2016

Just updated with Finder, let me know when to rebase, just keeping the old commit for now.

btw +19 −19 sloc on the finder vs glob (but +1 dependency) 👯

I still use glob because it returns an empty array when nothing matches, avoids testing is_file and I'd still have to use it to match {yml,yaml}. Let me know if you see some improvements that could be made!

@teohhanhui
Copy link
Contributor

👍

@dunglas dunglas merged commit f002cf5 into api-platform:master Jun 8, 2016
@dunglas
Copy link
Member

dunglas commented Jun 8, 2016

Thank you for this good work on XML and YAML support @soyuka!

@teohhanhui
Copy link
Contributor

teohhanhui commented Jun 13, 2016

@soyuka I do not think the resources: key should be at the top of the individual YAML config files... Same for XML (<resources>)...

@dunglas
Copy link
Member

dunglas commented Jun 15, 2016

@teohhanhui why? It's usual to have a root for config files like that isn't it? IIRC it's how its works for all Symfony config files.

@teohhanhui
Copy link
Contributor

@dunglas For Symfony Validator and Serializer, when using individual config files the topmost key is the FQCN.

@dunglas
Copy link
Member

dunglas commented Jun 15, 2016

ok! 👍 then

@soyuka soyuka deleted the changeconfigname branch September 9, 2016 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants