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

Fixed loading of namespaced gems and added BEXT_VERBOSE #18

Merged
merged 1 commit into from
May 25, 2016

Conversation

lzap
Copy link
Contributor

@lzap lzap commented Jun 9, 2014

This was never working on 0.4, regression from 0.3 version.

If you can bump upstream we would be pleased. Thanks!

@movitto
Copy link
Contributor

movitto commented Jun 14, 2014

Hey @lzap, ran through this, looks good. Just to clarify, this is to address the case where the dep cannot be required but the namespaced file is nil, correct? In that case we should not throw the strict_error. The error should still be thrown if the namespaced file is not nil but cannot be required.

The only blocker that I see on my end is a few of the specs are broken with this commit, namely the ones that test the current functionality. After those are changed/removed, don't have any issue w/ merging. @jguiditta thoughts?

A couple small style issues (not blockers). Would prefer adding a helper to the Output class that writes to stdout and use that instead of using 'puts' directly. Then we could easily override that method with another to use the rails logger or whatever other custom logging mechanism.

Also for consistency, perhaps the setting of the 'verbose' variable could be moved to a 'parse_env' helper (eg see the 'System' and 'Output' classes). Again just nitpicking :-)

@lzap
Copy link
Contributor Author

lzap commented Jun 30, 2014

Right, I totally forgot we have unit tests now. Helped to catch me the bug. Can we set Travis CI on this project now? ;-)

Fixed the missing strict error, moved the verbose thing to Output class. Good catch. Tests are green. TY

@domcleal
Copy link

I've tested this, it seems to fix the issue of loading namespaced gems (such as fog-core) seen on 0.4.0, which worked on 0.3.0.

Could this be re-reviewed please?

@domcleal domcleal mentioned this pull request May 25, 2016
@movitto movitto merged commit 9f5f818 into bundlerext:master May 25, 2016
@movitto
Copy link
Contributor

movitto commented May 25, 2016

OK, merged. Tests are currently broken due to incompatibilities with more recent versions of rspec but that's a separate issue.

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.

None yet

3 participants