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

Output underlying load error when wrapping #284

Conversation

timdiggins
Copy link
Contributor

Fixes #282. Depends on #283 for travis.

In the end it seemed cleaner (and safer) to create a tested utility-method for catching the LoadError and then reissuing with added information.

Currently in separate commits for ease of reading, but can squash /rebase if desired.

@gjtorikian
Copy link
Owner

This looks great, thank you!

I'm a bit suspicious of the Travis change; no other Ruby project I run has required a config change like that recently. What was the original error #283 was fixing?

@timdiggins
Copy link
Contributor Author

@gjtorikian

I think you're right to be suspicious, but I do think it (only partially -- see below) fixes the problem (without it I couldn't get travis to build even once)

Firstly note that it seems to have been a sporadic problem on master (some builds failed, some succeeded).

The failure that I think we're fixing here is:

$ bundle install --jobs=3 --retry=3 --deployment --path=${BUNDLE_PATH:-vendor/bundle}
    You are trying to install in deployment mode after changing
    your Gemfile. Run `bundle install` elsewhere and add the
    updated gemfiles/rails_5.gemfile.lock to version control.

e.g. https://travis-ci.org/jch/html-pipeline/jobs/265274259#L321

This removes the --deployment bundler arg (by overriding the default).
However I think the .lock files should also be removed... I was totally wrong about the idea that it was to do with trusty. This is still building on precise. I think the dist should be specified (given precise or moved to trusty) -- have moved to trusty and removed lockfiles in #285. Merge that and rebase?

@gjtorikian
Copy link
Owner

gjtorikian commented Sep 20, 2017

Could you merge master + rebase to remove e27f50a from this PR? I'm still not entirely sure bundler_args is necessary but if Travis fails without it, I'll deal with fixing it.

@timdiggins timdiggins force-pushed the output-underlying-load-error-when-wrapping branch from 4516d3f to b18a2a5 Compare September 21, 2017 15:15
@timdiggins
Copy link
Contributor Author

All done -- good to go!

@gjtorikian
Copy link
Owner

Fantastic, thanks!

@gjtorikian gjtorikian merged commit 010a6bd into gjtorikian:master Sep 21, 2017
@gjtorikian
Copy link
Owner

Out as 2.7.1.

@timdiggins timdiggins deleted the output-underlying-load-error-when-wrapping branch October 3, 2017 10:55
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