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

accept multiple YAML configurations using inherit parameter. #511

Merged
merged 2 commits into from
Apr 4, 2016

Conversation

kmuto
Copy link
Owner

@kmuto kmuto commented Apr 3, 2016

#503 の実装方針を変更し、includeを実装してみました。

ただ、includeは誤解を招く可能性があるため、inherit:という名前にしています。

@kmuto kmuto merged commit e5d9113 into master Apr 4, 2016
@takahashim
Copy link
Collaborator

出遅れてすみません、修正内容を見てみましたが、__がつくメソッド名は美しくないですし、そもそもクラスメソッドの再帰がOOPL的にあまり好ましくないように思いました。
ここはReVIEW::YAMLLoaderクラスを作って、YAMLLoader#loadとかで読み込むようにするのはどうでしょうか。

@takahashim takahashim mentioned this pull request Apr 4, 2016
@kmuto kmuto deleted the recursive_inherit_yaml branch April 8, 2016 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants