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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Configuration#dig() and ConfigView#dig() #1221

Merged
merged 1 commit into from Sep 22, 2017

Conversation

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Sep 21, 2017

Getting closer to winning your eternal gratitude 馃槃

@gpakosz gpakosz force-pushed the feature/config-dig branch from 6bda295 to fbb8f96 Sep 21, 2017

# @see Hash#dig
def dig(*keys)
@context.dependency_tracker.bounce(unwrap, attributes: keys)
Copy link
Member

@ddfreyne ddfreyne Sep 22, 2017

Choose a reason for hiding this comment

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

attributes is a list of top-level attributes that are touched, so this should be keys.first instead. Nanoc only tracks dependencies of keys one level deep.

(keys will also work, but generate unnecessary dependencies.)

Loading


context 'with non-existing keys' do
let(:keys) { %i[foo baz bar] }
it { is_expected.to eql(nil) }
Copy link
Member

@ddfreyne ddfreyne Sep 22, 2017

Choose a reason for hiding this comment

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

Prefer be_nil over eql(nil)

Loading

@ddfreyne ddfreyne merged commit fbb8f96 into nanoc:master Sep 22, 2017
3 checks passed
Loading
@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Sep 22, 2017

I鈥檝e applied the changes and merged it into master! Thanks for the quick contribution!

Loading

@gpakosz
Copy link
Member Author

@gpakosz gpakosz commented Sep 22, 2017

Thanks for the merge!

Loading

@gpakosz gpakosz deleted the feature/config-dig branch Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants