Skip to content

Conversation

geeknoid
Copy link
Contributor

@geeknoid geeknoid commented Feb 3, 2018

  • Was passing a incorrect directory when building the protoc-gen-doc plugin. Not sure how come this worked before,
    but the directory was definitely incrrect.

  • Sync to the latest doc gen tool.

@geeknoid geeknoid requested a review from rshriram as a code owner February 3, 2018 03:46
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 3, 2018
Makefile Outdated
@sed -e 's/*google_protobuf.Struct/interface{}/g' -e 's/ValueType_VALUE_TYPE_UNSPECIFIED/VALUE_TYPE_UNSPECIFIED/g' mixer/v1/config/cfg.pb.go >mixer/v1/config/fixed_cfg.pb.go
@sed -e 's/*google_protobuf.Struct/interface{}/g' \
-e 's/ValueType_VALUE_TYPE_UNSPECIFIED/VALUE_TYPE_UNSPECIFIED/g' \
-e 's/package config/package istio_mixer_v1_config/g' \
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think we want to do this. let's fix the package in istio/istio

@geeknoid
Copy link
Contributor Author

geeknoid commented Feb 3, 2018

Can we deal with the naming in a separate PR? I'm trying to get a bunch of doc work done, and the fact this is currently broken makes it so I can't do "go generate" on the protos in istio/istio. With this fix, the end to end pipeline is working.

@geeknoid geeknoid requested a review from xiaolanz as a code owner February 3, 2018 16:41
@douglas-reid
Copy link
Contributor

Let's use this: istio/istio#3146.

The package in istio/istio is not needed. And we will simplify the codegen.

@douglas-reid
Copy link
Contributor

@geeknoid we'll also need something like: #356.

@geeknoid geeknoid changed the title Correct a couple makefile issues Correct a small makefile issue Feb 3, 2018
@geeknoid
Copy link
Contributor Author

geeknoid commented Feb 3, 2018

PTAL

- Was passing a incorrect directory when building the protoc-gen-doc plugin. Not sure how come this worked before,
but the directory was definitely incrrect.

- Sync to the latest doc gen tool.
@geeknoid geeknoid merged commit 740b653 into istio:master Feb 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants