Skip to content
This repository has been archived by the owner on Mar 19, 2022. It is now read-only.

Automatically add Librarian's install directory to cookbook_paths #226

Merged
merged 7 commits into from
Apr 9, 2013

Conversation

tmatilai
Copy link
Collaborator

@tmatilai tmatilai commented Apr 7, 2013

No tests and all the path config should maybe be extracted to own class, but quite cool, isn't it? =)

@@ -107,7 +107,17 @@ def sync_kitchen
end

def cookbook_paths
Array(Chef::Config[:cookbook_path]) + [KnifeSolo.resource('patch_cookbooks').to_s]
unless @cookbook_paths
Copy link
Owner

Choose a reason for hiding this comment

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

Do we really need to memoize this? Seems like it might not be worth the code bloat. If you take out memoization and add a helper method this could be expanded_config_paths + [patch_cookbooks_path]. Or with the helper methods you could just add a ||= to memoize that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand and like the use of helper methods, but how could the dynamic adding work unless we store the updated list in an instance variable? Adding is done when running Librarian or Berkshelf, and later we use the list when rsyncing and generating solo.rb. Sorry if I'm not following...

@matschaffer
Copy link
Owner

Kinda neat how that works. Would love a test on it though.

@tmatilai
Copy link
Collaborator Author

tmatilai commented Apr 8, 2013

Will add tests. Just made the PR to get comments and that seemed to work great. Thanks. =)
This very same thing is later needed by Berkshelf, too.

@tmatilai
Copy link
Collaborator Author

tmatilai commented Apr 9, 2013

Extracted helpers and added tests.

@matschaffer
Copy link
Owner

Ooo... much nicer. I approve of this.

tmatilai added a commit that referenced this pull request Apr 9, 2013
Automatically add Librarian's install directory to cookbook_paths
@tmatilai tmatilai merged commit 02e923b into matschaffer:master Apr 9, 2013
@tmatilai tmatilai deleted the auto-add-librarian-dir branch April 9, 2013 21:02
matschaffer added a commit that referenced this pull request Apr 11, 2013
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.

2 participants