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

Configurable config loading #9

Merged
merged 2 commits into from Jan 18, 2016

Conversation

Projects
None yet
2 participants
@nawglan
Contributor

nawglan commented Jan 18, 2016

I believe this pull request addresses Issue #3 . It allows you to override all of the parameters passed to Config::Any with the exception of flatten_to_hash which I believe would break Config::Onion. To safeguard against this, I added a check to see if flatten_to_hash is passed in and remove it before passing to Config::Any.

nawglan added some commits Jan 18, 2016

Allow options to be passed to Config::Any
Added ability to override the default parameter of use_ext => 1 by
passing a hash ref to load and load_glob methods as the final parameter.
Updated README.mkdn to match POD
Changed the example method calls to match what is listed in the POD
@nawglan

This comment has been minimized.

Show comment
Hide comment
@nawglan

nawglan Jan 18, 2016

Contributor

Forgot to mention this is a PRC request. Thanks for participating.

Contributor

nawglan commented Jan 18, 2016

Forgot to mention this is a PRC request. Thanks for participating.

@dsheroh

This comment has been minimized.

Show comment
Hide comment
@dsheroh

dsheroh Jan 18, 2016

Owner

On Sun, Jan 17, 2016 at 09:03:21PM -0800, Desmond Daignault wrote:

Forgot to mention this is a PRC request. Thanks for participating.

I suspected as much. Thanks!

Dave Sherohman

Owner

dsheroh commented Jan 18, 2016

On Sun, Jan 17, 2016 at 09:03:21PM -0800, Desmond Daignault wrote:

Forgot to mention this is a PRC request. Thanks for participating.

I suspected as much. Thanks!

Dave Sherohman

dsheroh added a commit that referenced this pull request Jan 18, 2016

Merge pull request #9 from nawglan/master
Configurable config loading

@dsheroh dsheroh merged commit fe9cb9b into dsheroh:master Jan 18, 2016

@dsheroh

This comment has been minimized.

Show comment
Hide comment
@dsheroh

dsheroh Jan 18, 2016

Owner

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t .. 
<snip>
ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf. As far as I can tell, it seems to be a bug in Config::Any - I've verified that use_ext is set in that test and the Config::Any docs for use_ext say that it "will skip the file unless it matches a standard extension", not that it will die if it finds a file with an unknown extension. Upgrading to the latest Config::Any from CPAN (version 0.26) does not resolve this.

Do you have any ideas for a solution/workaround other than removing/renaming basic2.zzz? Also, if you don't get this test failure, what version is your Config::Any?

Owner

dsheroh commented Jan 18, 2016

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t .. 
<snip>
ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf. As far as I can tell, it seems to be a bug in Config::Any - I've verified that use_ext is set in that test and the Config::Any docs for use_ext say that it "will skip the file unless it matches a standard extension", not that it will die if it finds a file with an unknown extension. Upgrading to the latest Config::Any from CPAN (version 0.26) does not resolve this.

Do you have any ideas for a solution/workaround other than removing/renaming basic2.zzz? Also, if you don't get this test failure, what version is your Config::Any?

@nawglan

This comment has been minimized.

Show comment
Hide comment
@nawglan

nawglan Jan 19, 2016

Contributor

I forgot I was working on a patch for Config::Any. Please rename as
necessary. Basically, I was wanting it to be a yaml file with a non-yaml
extension.

Dez.

On Mon, Jan 18, 2016 at 4:00 AM, Dave Sherohman notifications@github.com
wrote:

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t ..

ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.

Tests were run but no plan was declared and done_testing() was not seen.

Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf.
As far as I can tell, it seems to be a bug in Config::Any - I've verified
that use_ext is set in that test and the Config::Any docs for use_ext say
that it "will skip the file unless it matches a standard extension",
not that it will die if it finds a file with an unknown extension.
Upgrading to the latest Config::Any from CPAN (version 0.26) does not
resolve this.

Do you have any ideas for a solution/workaround other than
removing/renaming basic2.zzz? Also, if you don't get this test failure,
what version is your Config::Any?


Reply to this email directly or view it on GitHub
#9 (comment).

Contributor

nawglan commented Jan 19, 2016

I forgot I was working on a patch for Config::Any. Please rename as
necessary. Basically, I was wanting it to be a yaml file with a non-yaml
extension.

Dez.

On Mon, Jan 18, 2016 at 4:00 AM, Dave Sherohman notifications@github.com
wrote:

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t ..

ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.

Tests were run but no plan was declared and done_testing() was not seen.

Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf.
As far as I can tell, it seems to be a bug in Config::Any - I've verified
that use_ext is set in that test and the Config::Any docs for use_ext say
that it "will skip the file unless it matches a standard extension",
not that it will die if it finds a file with an unknown extension.
Upgrading to the latest Config::Any from CPAN (version 0.26) does not
resolve this.

Do you have any ideas for a solution/workaround other than
removing/renaming basic2.zzz? Also, if you don't get this test failure,
what version is your Config::Any?


Reply to this email directly or view it on GitHub
#9 (comment).

@nawglan

This comment has been minimized.

Show comment
Hide comment
@nawglan

nawglan Jan 19, 2016

Contributor

I have a fix that works with current config::any. Just changed the test
for load_glob so that it specified force_plugins.

On Tue, Jan 19, 2016 at 1:16 PM, Nawglan nawglan@gmail.com wrote:

I forgot I was working on a patch for Config::Any. Please rename as
necessary. Basically, I was wanting it to be a yaml file with a non-yaml
extension.

Dez.

On Mon, Jan 18, 2016 at 4:00 AM, Dave Sherohman notifications@github.com
wrote:

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t ..

ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.

Tests were run but no plan was declared and done_testing() was not seen.

Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf.
As far as I can tell, it seems to be a bug in Config::Any - I've verified
that use_ext is set in that test and the Config::Any docs for use_ext
say that it "will skip the file unless it matches a standard extension",
not that it will die if it finds a file with an unknown extension.
Upgrading to the latest Config::Any from CPAN (version 0.26) does not
resolve this.

Do you have any ideas for a solution/workaround other than
removing/renaming basic2.zzz? Also, if you don't get this test failure,
what version is your Config::Any?


Reply to this email directly or view it on GitHub
#9 (comment).

Contributor

nawglan commented Jan 19, 2016

I have a fix that works with current config::any. Just changed the test
for load_glob so that it specified force_plugins.

On Tue, Jan 19, 2016 at 1:16 PM, Nawglan nawglan@gmail.com wrote:

I forgot I was working on a patch for Config::Any. Please rename as
necessary. Basically, I was wanting it to be a yaml file with a non-yaml
extension.

Dez.

On Mon, Jan 18, 2016 at 4:00 AM, Dave Sherohman notifications@github.com
wrote:

After merging, I'm getting test failures in t/02-load.t:

t/02-load.t ..

ok 7 - multiple config files loaded correctly
There are no loaders available for .zzz files at Config-Onion/lib/Config/Onion.pm line 91.

Tests were run but no plan was declared and done_testing() was not seen.

Looks like your test exited with 255 just after 7.

This appears to be triggered by the basic2.zzz file you added in t/conf.
As far as I can tell, it seems to be a bug in Config::Any - I've verified
that use_ext is set in that test and the Config::Any docs for use_ext
say that it "will skip the file unless it matches a standard extension",
not that it will die if it finds a file with an unknown extension.
Upgrading to the latest Config::Any from CPAN (version 0.26) does not
resolve this.

Do you have any ideas for a solution/workaround other than
removing/renaming basic2.zzz? Also, if you don't get this test failure,
what version is your Config::Any?


Reply to this email directly or view it on GitHub
#9 (comment).

@dsheroh

This comment has been minimized.

Show comment
Hide comment
@dsheroh

dsheroh Jan 20, 2016

Owner

Cool, the patch fixed the test. It really only needed force_plugins there, so I removed use_ext from the modified test.

Have you been in contact with Config::Any's maintainer yet about your patch? I'd definitely like to see use_ext actually working like the documentation says it does, so that it will be possible to point it at a directory containing a mix of config and non-config files and have it load all the configs while ignoring the other files.

Owner

dsheroh commented Jan 20, 2016

Cool, the patch fixed the test. It really only needed force_plugins there, so I removed use_ext from the modified test.

Have you been in contact with Config::Any's maintainer yet about your patch? I'd definitely like to see use_ext actually working like the documentation says it does, so that it will be possible to point it at a directory containing a mix of config and non-config files and have it load all the configs while ignoring the other files.

@nawglan

This comment has been minimized.

Show comment
Hide comment
@nawglan

nawglan Jan 20, 2016

Contributor

Yes, but no reply yet. Once I am happy with a patch, I will open a ticket
and supply the patch to Config::Any's maintainer.

Dez.

On Wed, Jan 20, 2016 at 3:05 AM, Dave Sherohman notifications@github.com
wrote:

Cool, the patch fixed the test. It really only needed force_plugins there,
so I removed use_ext from the modified test.

Have you been in contact with Config::Any's maintainer yet about your
patch? I'd definitely like to see use_ext actually working like the
documentation says it does, so that it will be possible to point it at a
directory containing a mix of config and non-config files and have it load
all the configs while ignoring the other files.


Reply to this email directly or view it on GitHub
#9 (comment).

Contributor

nawglan commented Jan 20, 2016

Yes, but no reply yet. Once I am happy with a patch, I will open a ticket
and supply the patch to Config::Any's maintainer.

Dez.

On Wed, Jan 20, 2016 at 3:05 AM, Dave Sherohman notifications@github.com
wrote:

Cool, the patch fixed the test. It really only needed force_plugins there,
so I removed use_ext from the modified test.

Have you been in contact with Config::Any's maintainer yet about your
patch? I'd definitely like to see use_ext actually working like the
documentation says it does, so that it will be possible to point it at a
directory containing a mix of config and non-config files and have it load
all the configs while ignoring the other files.


Reply to this email directly or view it on GitHub
#9 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment