Skip to content
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

Access custom config array throughout session #6200

Merged
merged 3 commits into from Aug 7, 2017

Conversation

@ashmaroli
Copy link
Member

ashmaroli commented Jul 3, 2017

Resolves #6180
Instead of having the array of custom configs removed, let it be available throughout current session.

Specific Use Case:

Allow Jekyll Admin to determine which config file to re-read if not the default _config.yml or _config.yaml

/cc @jekyll/ecosystem @parkr

Copy link
Member

parkr left a comment

I dig the idea.

Three things: (1) when config is empty/has an empty string like above, as checked in the line directly after the one you change, let's fill that in with whatever file we read by default; (2) let's add a test; (3) let's not expose this in liquid yet, so create a no-op method in SiteDrop called config.

@ashmaroli

This comment has been minimized.

Copy link
Member Author

ashmaroli commented Jul 3, 2017

Three things: (1) when config is empty/has an empty string like above, as checked in the line directly after the one you change, let's fill that in with whatever file we read by default; (2) let's add a test; (3) let's not expose this in liquid yet, so create a no-op method in SiteDrop called config.

(1) Isn't that how it is already? If config_files is an empty array, proceed to look for default config file(s)
(2) Fixing failing tests do produce enough results asserting the changed behavior, no clear on what else needs to be tested..
(3) Implemented with an empty #config

@parkr

@ashmaroli

This comment has been minimized.

Copy link
Member Author

ashmaroli commented Aug 7, 2017

Hi, Any other changes to be made?

@ghost
ghost approved these changes Aug 7, 2017
Copy link

ghost left a comment

this looks good to me

@ashmaroli

This comment has been minimized.

Copy link
Member Author

ashmaroli commented Aug 7, 2017

👍 Thanks Olivia

@parkr
parkr approved these changes Aug 7, 2017
@parkr

This comment has been minimized.

Copy link
Member

parkr commented Aug 7, 2017

👏

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit c8eee7f into jekyll:master Aug 7, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jekyllbot added a commit that referenced this pull request Aug 7, 2017
@ashmaroli ashmaroli deleted the ashmaroli:access-custom-configs branch Aug 7, 2017
@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.