Feature/new config system 2 recursive#1736
Closed
yanick wants to merge 27 commits into
Closed
Conversation
Role::Config is the configuration part of an object Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Remove everything that has to do with loading config from files or elsewhere. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Create Dancer2::Core::Role::ConfigReader and Dancer2::ConfigReader::File::Simple. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Also specify lazy => 1. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
* Remove previously commented out code * Remove 'use Data::Dumper' Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>
yanick
commented
Jul 12, 2025
| @@ -0,0 +1,220 @@ | |||
| # ABSTRACT: Config reader for files | |||
Contributor
Author
There was a problem hiding this comment.
Ooops, the rename wasn't caught. This shouldn't be here.
yanick
force-pushed
the
feature/new-config-system-2-recursive
branch
from
b565f92 to
1cf0d60
Compare
Contributor
|
\o/ 👍 This is a nice enhancement to the new config changes. |
bigpresh
approved these changes
Jul 18, 2025
bigpresh
left a comment
bigpresh
left a comment
Member
There was a problem hiding this comment.
A few minor tweaks suggested, but none of which feel like full blockers to me.
I do wonder if B<ConfigReader> should be C<ConfigReader> throughout, though - I'd have used the latter personally.
Comment on lines
+607
to
+608
| Outputs a lot of debugging information when generating the configuration of | ||
| the application. |
Member
There was a problem hiding this comment.
Do we mean "generating" here, or "reading"?
Comment on lines
+91
to
+92
| die "additional_config_readers entry can have only one key\n" | ||
| if 1 < keys %$thing; |
Member
There was a problem hiding this comment.
I think it'd be a bit more readable as if keys %$thing != 1, and it would also catch any case where we were passed an empty hashref too.
Suggested change
| die "additional_config_readers entry can have only one key\n" | |
| if 1 < keys %$thing; | |
| die "additional_config_readers entry must have exactly one key\n" | |
| if keys %$thing != 1; |
Contributor
There was a problem hiding this comment.
I agree. Yanick is AFK for the next few weeks, so I will update and merge today.
Comment on lines
+165
to
+167
| This class provides a C<config> attribute that | ||
| is populated by executing one or more B<ConfigReader> packages. | ||
| The default ConfigReader used by default is C<Dancer2::ConfigReader::Config::Any>. |
Member
There was a problem hiding this comment.
Slightly less tautology:
Suggested change
| This class provides a C<config> attribute that | |
| is populated by executing one or more B<ConfigReader> packages. | |
| The default ConfigReader used by default is C<Dancer2::ConfigReader::Config::Any>. | |
| This class provides a C<config> attribute which is populated by executing | |
| one or more B<ConfigReader> packages. | |
| The default ConfigReader used is L<Dancer2::ConfigReader::Config::Any>. |
Contributor
There was a problem hiding this comment.
Much better! I will make this change and merge today.
Contributor
|
Merged, including changes from @bigpresh. Thanks all! |
cromedome
added a commit
that referenced
this pull request
Sep 15, 2025
[ BUG FIXES ]
* GH #1701: Split cookie values on & only (Yanick Champoux)
[ ENHANCEMENTS ]
* GH #530: Make data censoring configurable (Yanick Champoux, David
Precious)
* GH #850: Scaffold tutorial app; allow multiple apps to be scaffolded
in core Dancer2 (Jason A. Crome)
* GH #1512: Log hook entries as they are executed (Yanick Champoux)
* GH #1615: Remove Dancer2::Template::Simple from Dancer2 core (Jason
A. Crome)
* PR #1637: New, extendable configuration system (Mikko Koivunalho)
* GH #1723: Enable use of a different Template Toolkit base class
(Andy Beverley)
* PR #1727: Don't create CPAN package files when generating new apps
(Jason A. Crome)
* PR #1731: Retire Template::Tiny fork, use CPAN's (Jason A. Crome,
Karen Etheridge, Damien Krotkine, Yanick Champoux)
* PR #1736: Allow config system to bootstrap itself (Yanick Champoux)
* GH #1737: Add on_hook_exception for errors during hook processing
(Andy Beverley)
* PR #1739: Add source to on_hook_exception (Andy Beverley)
* PR #1742: Refactor CLI for future expansion (Jason A. Crome)
[ DOCUMENTATION ]
* GH #1342: Document skipping private methods in pod coverage tests
(Jason A. Crome)
* PR #1721: New Dancer2 docs, reorganization of all documentation;
from TPRF grant (Jason A. Crome)
* PR #1741: Cover items missed in earlier documentation branch (Jason
A. Crome)
[ DEPRECATED ]
* None
[ MISC ]
* None
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow #1637 to bootstrap itself (i.e. have the base File::Simple config reader to instantiate and use subsequent config readers).
Will become undrafted when the parent PR is merged.