Update Geth to 1.9.0 #117
Update Geth to 1.9.0 #117
Conversation
| @@ -220,7 +220,7 @@ var _ = Describe("Parser", func() { | |||
| Expect(err).ToNot(HaveOccurred()) | |||
|
|
|||
| selectMethods := p.GetMethods([]string{}) | |||
| Expect(len(selectMethods)).To(Equal(22)) | |||
| Expect(len(selectMethods)).To(Equal(25)) | |||
There was a problem hiding this comment.
@i-norden it seems like the changes to Geth caused this function to return more methods - do you know if this is a problem?
There was a problem hiding this comment.
It seems they fixed something.. inspecting the ABI for that contract shows that there are 25 functions, not 22... Not sure what exactly was changed such that 3 of the functions that were previously not considered methods are now considered as such, but I don't think it is a problem either way. Will update if I find anything else out!
There was a problem hiding this comment.
Just noticed this is a WIP, I may have approved prematurely 🙃
rmulhol
requested review from
i-norden,
m0ar,
aaizuss,
elizabethengelman,
Gslaughl and
yaoandrew
|
I wasn't able to figure out how to use un-versioned dependencies with |
rmulhol
force-pushed
the
geth-1.9.0
branch
3 times, most recently
from
b37082d to
a2b2634
Compare
|
@i-norden Or do you mean you need dependencies without version? 🤔 |
|
Also this is really sexy 💯 🥇 |
|
Yeah I mean dependencies that aren't versioned. |
|
@i-norden why not just release a version of our fork? |
|
@rmulhol that's a good point, I'll just do that. I wasn't sure how version releases work on forks- those releases are only local to the fork and require no approval from the original repo maintainers? I see it being somewhat cumbersome having to do a new release of a dependency any time we want to test any changes we make to it- no matter how small- but I'll adjust! |
|
If the releases on a fork are local to that fork only, can E.g. normally a dependency is declared like
How can we change the source to the vulcanizedb repo to use a version there? |
|
@i-norden good question! I think the replace directive might greatly simplify things - looks like you can point to release on a fork, or even to the local filesystem. That said, I'm totally open to postponing the use of Also, if we are ultimately pointing at a release on a fork, I think we can pretty easily delete and replace them (if we don't want to clutter the fork repo during development). |
|
@rmulhol awesome! That replace directive is exactly the thing I need- thanks for pointing that out, and sorry for missing that! Don't postpone for me, I think all my concerns have been allayed. |
|
I'm able to build this with the go modules! However, I'm seeing a weird error when I run It looks to be trying to find an old configuration variable, but we don't have any references of that var in our current Other than that, I'm really into these changes but would like to take a closer. 😄 |
|
I can build this with go modules 😄 |
- set GO111MODULE=on - use go 1.12
This diff looks unnecessarily large because I deleted
vendor/🎉 🚀 ✂️Would appreciate if folks would give a go at checking out the branch and running
make build. The reason I deletedvendor/is that I was having trouble executingmake buildafter runninggo mod vendordue to.hand.cfiles not getting pulled in. If it works with versioned deps specified ingo.modand novendor/, I'm thinking we might be able to safely delete ~2 million lines of code.Important: these changes require go 1.12 and
export GO111MODULE=on