corcel / acf

Advanced Custom Fields (ACF) plugin for Corcel
127 stars 100 forks source link

Make it possible to define custom acf types #30

Open kevinkiel opened 7 years ago

kevinkiel commented 7 years ago

Example: corcel.config.php

corcel
jgrossi commented 7 years ago

Hi @kevinkiel thanks for contributing with Corcel ;-)

What do you think? JG

kevinkiel commented 7 years ago

@jgrossi Thanks for your fast reaction and feedback. I made some small changes to the code so it would pass the tests.

In the case of using a config file I think it's better to add a sample file in corcel/corcel repo, with a CorcelServiceProvider or something like that. Than the user can get a default file with the suggested options; I don't understand what you mean. This config file is specifically for the support of custom ACF plugins. Maybe add it in corcel/acf?

jgrossi commented 7 years ago

What I mean is to create the config/corcel.php file inside the corcel/acf merge an existent config/corcel.php in corcel/corcel. We can merge configurations between files. This way we would have only one config/corcel.php file.

Take a look: https://laravel.com/docs/5.3/packages#configuration

kevinkiel commented 7 years ago

@jgrossi Thanks for your response. I've add the CorcelAcfServiceProvider. Now my question is in what package are we gonna merge the 2 config files?

$this->mergeConfigFrom( __DIR__.'/config.php', 'corcel' );

kevinkiel commented 7 years ago

@jgrossi What do you think?

jgrossi commented 7 years ago

Hi @kevinkiel sorry for being MIA but time is scarce for now :-)

My goal is to have, at least for now, just one config/corcel.php file. I'm rewriting Corcel (you're welcome to help btw) and it'll have a config/corcel.php file, just to set the default Corcel connection, nothing more for now, I guess.

I was wondering if I can merge the config options you created (about acf) into the main config/corcel.php file, what I guess it's possible, but I've never tried that.

I'll try to install a fresh Laravel here, install Corcel, copy the config file and then install corcel/acf and try to merge it. I let you know if it's working and then we merge this PR, which I think is ready.

Thanks!