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

Create config files in config_dir #265

Merged
merged 3 commits into from
Feb 2, 2016

Conversation

gdavison
Copy link
Contributor

So that they can be found and loaded by the Consul agent

@codecov-io
Copy link

Current coverage is 68.61%

Merging #265 into master will increase coverage by +13.27% as of 9e71b9f

@@            master    #265   diff @@
======================================
  Files            7       7       
  Stmts          318     376    +58
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            176     258    +82
  Partial          0       0       
+ Missed         142     118    -24

Review entire Coverage Diff as of 9e71b9f

Powered by Codecov. Updated on successful CI builds.

@Ginja
Copy link
Contributor

Ginja commented Jan 28, 2016

👍 good catch.

provides(:consul_watch)
default_action(:create)

# @!attribute path
# @return [String]
attribute(:path, kind_of: String, default: lazy { "/etc/consul/#{name}.json" })
attribute(:path, kind_of: String, default: lazy { join_path node['consul']['service']['config_dir'], "#{name}.json" })

This comment was marked as outdated.

@johnbellone
Copy link
Contributor

Take a look at my above comments. Let's get this merged and I think this is the last big obstacle for the 1.4 release.

@johnbellone johnbellone mentioned this pull request Jan 28, 2016
@gdavison
Copy link
Contributor Author

I favoured the node value for a couple reasons:

  1. We only have to set the default in one place
  2. If someone wants to override the default, they don't have to add a path value to every consul resource they create

Not a huge issue to me either way, though.

johnbellone added a commit that referenced this pull request Feb 2, 2016
Create config files in config_dir
@johnbellone johnbellone merged commit c8b2f4b into sous-chefs:master Feb 2, 2016
@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants