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

Drop verification of existing firebase-tools installation #713

Closed
wants to merge 1 commit into from
Closed

Drop verification of existing firebase-tools installation #713

wants to merge 1 commit into from

Conversation

sylveon
Copy link

@sylveon sylveon commented Nov 11, 2017

This prevents travis-ci/travis-ci#8637 from happening.

The check uses the PATH, which in case of node.js projects includes node_modules/.bin. If the firebase-tools package has been installed locally, the check will pass because they are stored in said folder, so the tools will not get installed globally. The problem is that when the deploy is run, node_modules/.bin isn't in the PATH so it results in 'sh: 1: firebase: not found' when deploying.

This prevents travis-ci/travis-ci#8637 from happening.

The check uses the PATH, which in case of node.js projects includes `node_modules/.bin`. If the firebase-tools package has been installed locally, the check will pass because they are stored in said folder, so the tools will not get installed globally. The problem is that when the deploy is run, `node_modules/.bin` isn't in the PATH so it results in 'sh: 1: firebase: not found' when deploying.
@BanzaiMan
Copy link
Contributor

BanzaiMan commented Nov 13, 2017

I have a feeling that this is not the right fix. If the mismatch between RVM's PATH and Kernel#`'s PATH is a general problem (not just Firebase), it should be fixed in DPL::Provider.

@sylveon
Copy link
Author

sylveon commented Nov 20, 2017

I mean, that'd be ideal, sure. But then either:

@BanzaiMan
Copy link
Contributor

The version specified in this PR is too old at this point.

@BanzaiMan BanzaiMan closed this Aug 12, 2019
@svenfuchs
Copy link
Contributor

Hey @sylveon 👋

We are working on releasing a new major version dpl v2. The respective branch has been merged to master, and we are planning on releasing a developer preview, as well as a blog post soon, hopefully later this week.

I believe this PR here addresses your issue by adding node_modules/.bin to the PATH #1033. I believe this should be an adequate fix, what do you think?

Thank you!

@sylveon
Copy link
Author

sylveon commented Aug 20, 2019

It should, but I don't think you linked the right PR, as it's completely unrelated to node.

@svenfuchs
Copy link
Contributor

@sylveon lol, sorry about that. juggling too many PRs at the moment. here's the right one: #1032

svenfuchs added a commit that referenced this pull request Aug 21, 2019
firebase: add node_modules/.bin to the PATH (ports #713)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants