(PDK-668) Templatedir now reads .sync.yml for config when rendering t… #354
Conversation
HelenCampbell
force-pushed
the
usesyncyaml
branch
from
2cd9a3a to
97e1005
Compare
HelenCampbell
changed the title
HelenCampbell
force-pushed
the
usesyncyaml
branch
from
97e1005 to
9305903
Compare
lib/pdk/module/templatedir.rb
Outdated
| @config = {} | ||
| end | ||
| if @config.nil? | ||
| confdefaults = read_config(config_path) |
There was a problem hiding this comment.
Tiny nitpick here, but just for consistency sake, confdefaults and syncconfig should be underscore separated like the other variables are.
lib/pdk/module/templatedir.rb
Outdated
| file_config = @config.fetch(:global, {}) | ||
| file_config['module_metadata'] = @module_metadata | ||
| file_config.merge!(@config.fetch(dest_path, {})) unless dest_path.nil? | ||
| file_config.merge!(@config) | ||
| file_config |
There was a problem hiding this comment.
This line is no longer needed since the last line is no longer conditional.
| YAML.load(config_defaults) # rubocop:disable Security/YAMLLoad | ||
| end | ||
|
|
||
| before(:each) do |
There was a problem hiding this comment.
The config_defaults related allows should be moved up to the top level before block for maintenance purposes since they are required to be mocked with or without a sync.yml file.
lib/pdk/module/templatedir.rb
Outdated
| @@ -236,26 +232,44 @@ def self.files_in_template(dirs) | |||
| # | |||
| # @api private | |||
| def config_for(dest_path) | |||
There was a problem hiding this comment.
It would be cool if the path to sync.yml could be passed in as an optional parameter here, and if it is nil, then the code below can default the path to the <dest_path>/sync.yml. This will allow us to potentially add support for externally provided sync.yml config in the future.
lib/pdk/module/templatedir.rb
Outdated
| @@ -236,26 +232,44 @@ def self.files_in_template(dirs) | |||
| # | |||
| # @api private | |||
| def config_for(dest_path) | |||
| if @config.nil? | |||
| config_path = File.join(@path, 'config_defaults.yml') | |||
| sync_config_path = File.join(@path, '/.sync.yml') | |||
There was a problem hiding this comment.
The / isn't necessary on a File.join. It will actually cause problems on Windows.
HelenCampbell
force-pushed
the
usesyncyaml
branch
from
9305903 to
13f2414
Compare
HelenCampbell
changed the title
|
@bmjen Updated with your changes! |
…emplates.