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

Allow specifying the `path` option to bundler #165

Merged
merged 1 commit into from Feb 15, 2020
Merged

Conversation

@nickcharlton
Copy link
Member

nickcharlton commented Feb 10, 2020

The path option is often used to cache dependencies on CI, and this
causing projects like Administrate to do unnecessary work.

It's important that we pass the absolute file path to --path, as
otherwise it's assumed to be inside the gemfiles directory when run.

@nickcharlton nickcharlton force-pushed the nc-install-path branch 2 times, most recently from db10096 to 4d1ff00 Feb 11, 2020
nickcharlton added a commit to thoughtbot/administrate that referenced this pull request Feb 11, 2020
Previously, `appraisal install` didn't pass through a `path` argument.
[This is in progress][1]. [This continues the work started
previously.][2]

So far, this duplicates the existing cache restore/set directives to see
if we get the behaviour intended.

[1]: thoughtbot/appraisal#165
[2]: #1482
nickcharlton added a commit to thoughtbot/administrate that referenced this pull request Feb 12, 2020
Previously, `appraisal install` didn't pass through a `path` argument.
[This is in progress][1]. [This continues the work started
previously.][2]

So far, this duplicates the existing cache restore/set directives to see
if we get the behaviour intended.

[1]: thoughtbot/appraisal#165
[2]: #1482
Copy link
Member

mike-burns left a comment

LGTM overall, just one minor idea.

lib/appraisal/appraisal.rb Outdated Show resolved Hide resolved
lib/appraisal/appraisal.rb Show resolved Hide resolved
The `path` option is often used to cache dependencies on CI, and this
causing projects like [Administrate][1] to do unnecessary work.

It's important that we pass the absolute file path to `--path`, as
otherwise it's assumed to be inside the `gemfiles` directory when run.

[1]: thoughtbot/administrate#1482
@nickcharlton nickcharlton force-pushed the nc-install-path branch from 4d1ff00 to a3b9300 Feb 15, 2020
@nickcharlton

This comment has been minimized.

Copy link
Member Author

nickcharlton commented Feb 15, 2020

Thanks @mike-burns!

@nickcharlton nickcharlton merged commit 659b6b7 into master Feb 15, 2020
3 checks passed
3 checks passed
Hound Smells good to me. Woof!
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@nickcharlton nickcharlton deleted the nc-install-path branch Feb 15, 2020
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

2 participants
You can’t perform that action at this time.