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

Convert project to glide. #419

Merged
merged 1 commit into from
Apr 26, 2016
Merged

Convert project to glide. #419

merged 1 commit into from
Apr 26, 2016

Conversation

jrick
Copy link
Member

@jrick jrick commented Apr 19, 2016

Fixes #399.

@jrick jrick force-pushed the glide branch 22 times, most recently from 89684bb to fece438 Compare April 20, 2016 03:34
@jrick
Copy link
Member Author

jrick commented Apr 20, 2016

Once this is in and btcd is converted to use glide as well, I would like to switch away from our btcsuite package paths back to the upstream ones. We can still maintain forked repos as a backup in case they disappear, but there is no reason to keep rewriting the import paths when we maintain control over the versions in the vendor directory.

This should be done after btcd also is converted to glide since we don't want to import the same package multiple times with different names, as some of these dependencies are shared in btcd and btcwallet code.

@jcvernaleo
Copy link
Member

Could you maybe point out in the readme that go commands that do not recurse (go build/install without the ./...) work as normal after the glide install so there is no need for the glide novendor in those cases.
More specifically, when I cross compile, I cannot use glide novendor. This does not work properly:
env GOOS=openbsd GOARCH=amd64 go build $(glide novendor)
while this is fine:
env GOOS=openbsd GOARCH=amd64 go build

@jcvernaleo
Copy link
Member

Other than that doc request this looks good to me and works as expected on Linux and BItrig.

@jrick
Copy link
Member Author

jrick commented Apr 20, 2016

sure, i'll update the readme to include both instructions

Since those instructions are the same for both first time installs and updates, i'll try to merge them into a single place too.

@jcvernaleo
Copy link
Member

Do I need to test this on OSX as well?

@jrick
Copy link
Member Author

jrick commented Apr 20, 2016

Unlikely to be an issue but it wouldn't hurt.

@jrick jrick force-pushed the glide branch 4 times, most recently from ecfe621 to 0b9c460 Compare April 20, 2016 18:33
@jcvernaleo
Copy link
Member

I tested and as expected, OSX was fine. And new instructions look good. Not sure if anyone else wanted to chime in but OK from me.


- **Glide**

Glide is used to manage project dependencies and provide reproducable builds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reproducible

@davecgh
Copy link
Member

davecgh commented Apr 26, 2016

It appears that everything has been addressed.

OK

@jrick jrick merged commit 9109336 into btcsuite:master Apr 26, 2016
@jrick jrick deleted the glide branch April 26, 2016 17:28
jrick added a commit to jrick/btcwallet that referenced this pull request May 6, 2016
In btcsuite#419 (comment)
I mentioned that btcwallet should wait until btcd also switches back
to upstream package paths so that duplicate packages are not included
in and bloat the btcwallet binary.  Even while btcd has not yet
switched to the upstream flags package, this change can still be made
now since it is only used by btcd's main package, which is not
included in btcwallet.
jrick added a commit to jrick/btcwallet that referenced this pull request May 6, 2016
In btcsuite#419 (comment)
I mentioned that btcwallet should wait until btcd also switches back
to upstream package paths so that duplicate packages are not included
in and bloat the btcwallet binary.  Even while btcd has not yet
switched to the upstream flags package, this change can still be made
now since it is only used by btcd's main package, which is not
included in btcwallet.
jrick added a commit to jrick/btcwallet that referenced this pull request Dec 5, 2016
This prevents future calls to CreateNewWallet from erroring due to the
database file existing.

Fixes btcsuite#418.
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.

3 participants