search

containerd / ttrpc

GRPC for low-memory environments
Apache License 2.0
558 stars 80 forks source link

Fix method full name generation #44

Closed kevpar closed 5 years ago

kevpar commented 5 years ago

Signed-off-by: Kevin Parsons kevpar@microsoft.com

crosbymichael commented 5 years ago

So prior to this change the method names where //something?

kevpar commented 5 years ago

@crosbymichael Yeah, the names were generated with a double "//" at the start. Maybe this was meant to use strings.Join instead of path.Join originally.

crosbymichael commented 5 years ago

Have you testing this for backwards compat? A shim compiled with the existing ttrpc and a daemon with this new ttrpc?

kevpar commented 5 years ago

It looks like this value is only used in interceptors, so I don't think there is any compat burden. Are there any interceptors already written that I should check? I'm not aware of any.

crosbymichael commented 5 years ago

I don't think so, it should be good then

kevpar commented 5 years ago 
$ go test -race -v ./...

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received
The build has been terminated

The build appears to have timed out. Is there a way to rerun it?

jterry75 commented 5 years ago

@dmcgowan - This failure is no way related to this change. Can we restart the tests?

dmcgowan commented 5 years ago

I am running again but seems stuck again

dmcgowan commented 5 years ago

My guess is the lack of vendoring is causing some issue

kevpar commented 5 years ago

@dmcgowan Is there anything else you need from me here? I don't think anything in this PR could be causing the CI failure.

crosbymichael commented 5 years ago

LGTM

Tested this locally, i'll look into the travis issues today. Looks vndr related