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

build: multiple cleanups #1575

Merged
merged 3 commits into from
May 15, 2020
Merged

build: multiple cleanups #1575

merged 3 commits into from
May 15, 2020

Conversation

dajohi
Copy link
Member

@dajohi dajohi commented May 13, 2020

build: replace travis-ci with github actions.
build: update deps
build: clean linter warnings

@jcvernaleo
Copy link
Member

Nice! I've wanted this for a while and haven't had time to do it myself.

I think either @davecgh or @Roasbeef needs to actually enable actions and disable travis for this to go in, but big +1 on it.

Copy link
Member

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

OK assuming action passes.

@dajohi
Copy link
Member Author

dajohi commented May 13, 2020

It will activate on merge.
https://github.com/dajohi/btcd/actions/runs/103580512

@jcvernaleo
Copy link
Member

Ah, cool. Will still need one of those guys to fix the branch protections though.

@dajohi
Copy link
Member Author

dajohi commented May 13, 2020

You'll have to disable the continuous-intergration/travis-ci requirement first.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Very cool! We're also starting the process of switching over to lnd ourselves as well. No major comments, just a few questions.

@@ -271,7 +271,7 @@ type GetBlockTemplateResult struct {

type MempoolFees struct {
Base float64 `json:"base"`
Modified float64 `json:"base"`
Modified float64 `json:"modified"`
Copy link
Member

Choose a reason for hiding this comment

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

Good catch, I think this had been around since the struct was introduced.

github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo=
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d h1:yJzD/yFppdVCf6ApMkVy8cUxV0XrxdP9rVf6D87/Mng=
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
github.com/btcsuite/btcutil v1.0.2 h1:9iZ1Terx9fMIOtq1VrwdqfsATL9MC2l8ZrUY6YZ2uts=
Copy link
Member

Choose a reason for hiding this comment

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

Do the prior ones go away with a go mod tidy?

github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd
github.com/btcsuite/goleveldb v0.0.0-20160330041536-7834afc9e8cd
github.com/btcsuite/snappy-go v0.0.0-20151229074030-0bdef8d06723 // indirect
github.com/btcsuite/goleveldb v1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Given modules exist now, do we still need to vendor goleveldb?

Copy link
Member

Choose a reason for hiding this comment

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

Our fork looks to be a year or so behind mainline: https://github.com/syndtr/goleveldb

@Roasbeef
Copy link
Member

Yeah once this is merged in, I can modify which status checks we require for merges.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 💫

@Roasbeef Roasbeef merged commit 9a88e1d into btcsuite:master May 15, 2020
@Roasbeef
Copy link
Member

Now on by default!

@Rjected
Copy link
Collaborator

Rjected commented May 26, 2020

@Roasbeef do you think you could remove the travis status check now that this has been merged?

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.

4 participants