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

Let's hardwire default paths to Gaia. #784

Closed
NodeGuy opened this issue Jun 1, 2018 · 10 comments · Fixed by #1021
Closed

Let's hardwire default paths to Gaia. #784

NodeGuy opened this issue Jun 1, 2018 · 10 comments · Fixed by #1021
Assignees

Comments

@NodeGuy
Copy link
Contributor

NodeGuy commented Jun 1, 2018

#748 builds Gaia binaries into builds/gaia. If BINARY_PATH and NODE_BINARY_PATH aren't set then let's have Voyager look for them there, or even better let's hardwire the paths and get rid of BINARY_PATH and NODE_BINARY_PATH.

@faboweb
Copy link
Collaborator

faboweb commented Jun 2, 2018 via email

@NodeGuy
Copy link
Contributor Author

NodeGuy commented Jun 3, 2018

I can't think of a reason. If a dev wants to use a different version of Gaia then he could specify a different version to be built.

@nylira
Copy link
Contributor

nylira commented Jun 4, 2018

The fewer configuration flags the better, I support this idea.

@jbibla
Copy link
Collaborator

jbibla commented Jun 19, 2018

i'd like us to revisit this!

@faboweb
Copy link
Collaborator

faboweb commented Jun 20, 2018

What do you mean? What do you propose?

@jbibla
Copy link
Collaborator

jbibla commented Jun 21, 2018

to run voyager i have to use this command:

BINARY_PATH=builds/Gaia/darwin_amd64/gaiacli NODE_BINARY_PATH=builds/Gaia/darwin_amd64/gaiad yarn testnet

but i'd like to be able to use this command:

yarn testnet

i think we're defaulting to local gaia instead of the gaia we're building with yarn build:gaia.

@faboweb
Copy link
Collaborator

faboweb commented Jun 22, 2018

👍 OK I got it. This should be an easy fix. The code handling the path is here: https://github.com/cosmos/voyager/blob/develop/app/src/main/index.js#L171

@jbibla jbibla added the discuss label Jun 29, 2018
@mappum mappum self-assigned this Jul 10, 2018
@NodeGuy NodeGuy self-assigned this Jul 10, 2018
@okwme okwme removed the discuss label Jul 10, 2018
@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018

I am sorry to bring this up again. But currently we the situation that some issues work on are on an old SDK version while others are on a newer SDK version. Having fixed paths for the binary would remove the ability to switch quickly between two SDK versions. Therefor I strongly disagree with getting rid of the binary path environment variables.

@okwme
Copy link
Contributor

okwme commented Jul 12, 2018

I thought this was w regard to letting the default gaia path point to the app-specific build, but still allowing setting the path explicitly for the scenario @faboweb outlined

@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018 via email

faboweb added a commit that referenced this issue Jun 2, 2020
* fix two bugs. title and repetition. some lint O.o

* changelog

* linted

* Update lib/notifications.js

* added s

* linted

Co-authored-by: Fabian Weber <[email protected]>
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 a pull request may close this issue.

6 participants