Use github.com/golang/protobuf/descriptor ForMessage and fix CI from not rebasing #857
Conversation
…cting FileDescriptorProto ourselves and fixing generated stream.swagger.json
Gopkg.toml
Outdated
| @@ -1,4 +1,5 @@ | |||
| required = [ | |||
| "github.com/golang/protobuf/descriptor", | |||
There was a problem hiding this comment.
You shouldn't need to explicitly add this as we're importing it.
There was a problem hiding this comment.
you would think wouldn't you:
"github.com/golang/protobuf/descriptor" is not imported by your project, and has been temporarily added to Gopkg.lock and vendor/. If you run "dep ensure" again before actually importing it, it will disappear from Gopkg.lock and vendor/.
There was a problem hiding this comment.
That doesn't sound right at all
There was a problem hiding this comment.
agreed, maybe it's because there are no go files in the root of the repo? dep is weird sometimes...
There was a problem hiding this comment.
I don't think that should matter. I've never had to add a package explicitly like this. Let me try checking out this branch.
There was a problem hiding this comment.
huh you're right, if I get rid of the requires everything still works fine, I guess I just confused dep by doing dep ensure -add github.com/golang/protobuf/descriptor
|
Hm, I figured our CI tests would have flagged that up 🤔. |
By this I mean the function where we set the sourceCodeInfo explicitly before. |
yeah good catch, removed |
Codecov Report
@@ Coverage Diff @@
## master #857 +/- ##
=========================================
+ Coverage 52.98% 53.08% +0.1%
=========================================
Files 39 39
Lines 3905 3888 -17
=========================================
- Hits 2069 2064 -5
+ Misses 1636 1630 -6
+ Partials 200 194 -6
Continue to review full report at Codecov.
|
considering another change that was merged before my other one changed the ABitOfEverything message and we had never generated the stream.swagger.json before my change I would have been impressed if the CI picked up this issue before the merge into master |
|
Thanks for this, I will confirm master builds successfully and release 1.7.0 🎉 |
…not rebasing (grpc-ecosystem#857) * Use github.com/golang/protobuf/descriptor ForMessage instead of extracting FileDescriptorProto ourselves and fixing generated stream.swagger.json * Fixing bazel and removing dead code * remove required github.com/golang/protobuf/descriptor * revert removing github.com/go-resty/resty
@johanbrandhorst this is the fix because I didn't rebase on master before you merged, also stumbled on https://godoc.org/github.com/golang/protobuf/descriptor as a replacement for the function I built to extract Descriptor