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

Added parent_config_file support in configuration file #419

Merged
merged 1 commit into from Apr 28, 2014

Conversation

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Apr 13, 2014

This replaces #414.


# Merge config with default config
@config = DEFAULT_CONFIG.merge(@config)

Copy link
Member

@ddfreyne ddfreyne Apr 16, 2014

Choose a reason for hiding this comment

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

This set of changes is quite long and should be extracted into its own method. A method that recursively loads config files should do the trick.

Loading

@gpakosz
Copy link
Member Author

@gpakosz gpakosz commented Apr 16, 2014

How about now?

Loading

raise Nanoc::Errors::GenericTrivial, "Cycle detected. Could not use parent configuration file '#{parent_config_file}'"
end
parent_config = load_config(config_path)
apply_parent_config(parent_config, config_paths << config_path).merge(config)
Copy link
Member

@ddfreyne ddfreyne Apr 16, 2014

Choose a reason for hiding this comment

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

config_paths + config_path rather than << because the latter mutates.

Loading

YAML.load_file(config_path).symbolize_keys_recursively
end

def apply_parent_config(config, config_paths = [])
Copy link
Member

@ddfreyne ddfreyne Apr 28, 2014

Choose a reason for hiding this comment

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

I’d prefer config_paths to be a Set rather than an array (there is no need to have it ordered).

A better name for this param would be seen_config_paths.

Loading

ddfreyne added a commit that referenced this issue Apr 28, 2014
Added parent_config_file support in configuration file
@ddfreyne ddfreyne merged commit df88011 into nanoc:master Apr 28, 2014
1 check passed
Loading
@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Apr 28, 2014

It’s fine!

Loading

@gpakosz gpakosz deleted the parent_config_file-support branch Jul 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants