-
Notifications
You must be signed in to change notification settings - Fork 2
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
clean up the csv import functionality #244
Conversation
jmartin-sul
commented
Sep 11, 2018
- centralize this shared functionality to a file in app/lib, move tests to corresponding spec file, have everything that needs the functionality use the app/lib method
- give it a slighty better name
- stop delegating unnecessarily in Bundle, and stop calling it via Bundle. remove duplicate test in bundle_spec.rb
- excise unnecessary redefinition and tests in bundle_context_temporary and its _spec
- switch to using the more standard ArgumentError instead of the custom BundleUsageError
Pull Request Test Coverage Report for Build 747
💛 - Coveralls |
* centralize this shared functionality to a file in app/lib, move tests to corresponding spec file, have everything that needs the functionality use the app/lib method * give it a slighty better name * stop delegating unnecessarily in Bundle, and stop calling it via Bundle. remove duplicate test in bundle_spec.rb * excise unnecessary redefinition and tests in bundle_context_temporary and its _spec * switch to using the more standard ArgumentError instead of the custom BundleUsageError
3cedc28
to
93cb0b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was going to be a private method in BundleContext, which is the only user? Is it split out because we're getting rid of BundleContextTemporary and this makes that easier?
when i went to make it a private method, it turned out there was one other consumer. the consumer was not so my comment on the last PR was wrong, it can't be a private method since smpl.rb still uses it (via |
re: |
fair nuff. |