Skip to content

Fixed namespace issue + added some infrastructure#1

Open
renra wants to merge 2 commits intoniklasb:masterfrom
renra:fix_namespace_issue
Open

Fixed namespace issue + added some infrastructure#1
renra wants to merge 2 commits intoniklasb:masterfrom
renra:fix_namespace_issue

Conversation

@renra
Copy link
Copy Markdown

@renra renra commented Jul 11, 2016

Summary of changes:

  • Test coverage

  • Fixed one namespace problem that I found when I was playing around with the original library. The following call to LookupStack is already outside of the DynamicBinding module and so it needs to be referenced like this: DynamicBinding::LookupStack

    https://github.com/niklasb/ruby-dynamic-binding/blob/master/lib/dynamic_binding.rb#L61

  • Split the implementation into two files so that users can choose what to import (for example not everybody might want to monkey-patch Proc). Requiring just dynamic_binding will require everything - that is reverse-compatible - but I could require just dynamic_binding/lookup_stack if I wanted

@niklasb
Copy link
Copy Markdown
Owner

niklasb commented Jul 12, 2016

Wow, great work! Didn't know anybody was actually still using this :) Would you mind giving a quick overview of what your changes are? I haven't touched Ruby for like 5 years... Looks pretty good though!

@renra
Copy link
Copy Markdown
Author

renra commented Jul 13, 2016

Hey @niklasb Your implementation saved my day actually! I have put the explanation to the PR description. I kept polishing up further and ended up with this simplified version which is all that I need. I published it to rubygems as contextual_proc in case you might want to claim dynamic_binding for your original implementation.

Thanks for putting this together!

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