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

Require bundler so that Bundler::GemfileNotFound becomes available #464

Merged

Conversation

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Jul 20, 2014

Without requiring bundler, Bundler::GemfileNotFound does not exist.

Sorry for the delay in fixing this. :(

CC @bobthecow @gpakosz @jugglinmike - 馃憤 if you like (picking some random people to review)

@coveralls
Copy link

@coveralls coveralls commented Jul 20, 2014

Coverage Status

Coverage increased (+0.2%) when pulling 5b21655 on require-bundler-for-gemfile-not-found-error into f859105 on release-3.7.x.

Loading

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Jul 20, 2014

馃憤

Loading

@bobthecow
Copy link
Member

@bobthecow bobthecow commented Jul 20, 2014

lgtm 馃憤

Loading

@FredyFreshFirm
Copy link

@FredyFreshFirm FredyFreshFirm commented Jul 21, 2014

Ok
20. juli 2014 18:18 skrev "Justin Hileman" notifications@github.com
f酶lgende:

lgtm [image: 馃憤]


Reply to this email directly or view it on GitHub
#464 (comment).

Loading

@jugglinmike
Copy link
Contributor

@jugglinmike jugglinmike commented Jul 22, 2014

I verified this manually, but I'm wondering if we can back this with a test. test/test_gem.rb seems a likely candidate. It probably requires a lot more infrastructure than that file provides (we'd have to temporarily move this project's own Gemfile).

It's likely more appropriate to open a separate issue to create tests for nanoc's executable. Does this seem like a good idea?

Loading

@FredyFreshFirm
Copy link

@FredyFreshFirm FredyFreshFirm commented Jul 22, 2014

Yup

Fredy Ot茅n
22. juli 2014 14:23 skrev "jugglinmike" notifications@github.com f酶lgende:

I verified this manually, but I'm wondering if we can back this with a
test. test/test_gem.rb
https://github.com/nanoc/nanoc/blob/db1db138608d4f6786e1a2482f12ab20359b424c/test/test_gem.rb
seems a likely candidate. It probably requires a lot more infrastructure
than that file provides (we'd have to temporarily move this project's own
Gemfile).

It's likely more appropriate to open a separate issue to create tests for
nanoc's executable. Does this seem like a good idea?


Reply to this email directly or view it on GitHub
#464 (comment).

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Jul 22, 2014

@jugglinmike The problem is that we already use Bundler to run the tests, which means bundler is already required. bin/nanoc is not tested except manually.

Loading

@jugglinmike
Copy link
Contributor

@jugglinmike jugglinmike commented Jul 22, 2014

Our dependence on Bundler is no problem for testing this change--it just needs to ensure that the executable functions correctly in the absence of a Gemfile. But I don't mean to bikeshed; you should open a dedicated issue if you think that's worthwhile.

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Aug 9, 2014

@jugglinmike It鈥檚 very hard to test properly because it depends on the gems you have installed. You鈥檇 need to have a way of automatically testing with different gem setups (no Bundler + no Gemfile, no Bundler + Gemfile, Bundler + no Gemfile, Bundler + Gemfile).

Loading

ddfreyne added a commit that referenced this issue Aug 9, 2014
鈥nd-error

Require bundler so that Bundler::GemfileNotFound becomes available
@ddfreyne ddfreyne merged commit 9eb14fd into release-3.7.x Aug 9, 2014
1 check passed
Loading
@ddfreyne ddfreyne deleted the require-bundler-for-gemfile-not-found-error branch Aug 9, 2014
@ddfreyne ddfreyne added the bug label Aug 17, 2014
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

6 participants