Skip to content

Conversation

guptasu
Copy link
Contributor

@guptasu guptasu commented Feb 16, 2018

  • move cfg.proto and valuetype.proto
  • please double check that the proto comments + references are now using new location.
  • also please check the package names + $ comments for docgen.

@guptasu guptasu requested a review from rshriram as a code owner February 16, 2018 02:17
@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 16, 2018
Makefile Outdated
@@ -214,12 +214,18 @@ mixer_adapter_model_v1beta_protos := $(shell find $(mixer_adapter_model_v1beta_p
mixer_adapter_model_v1beta_pb_gos := $(mixer_adapter_model_v1beta_protos:.proto=.pb.go)
mixer_adapter_model_v1beta_pb_doc := $(mixer_adapter_model_v1beta_path)/istio.mixer.adapter.model.v1beta.pb.html

policy_v1beta_path := policy/v1beta
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn’t it be v1beta1?

@@ -0,0 +1,214 @@
// Copyright 2016 Istio Authors
Copy link
Member

Choose a reason for hiding this comment

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

2018
Also can we use this opportunity to name this file to something more seo friendly ? I believe this is a key piece of mixer reference docs. So it could use a more friendly name.

Copy link
Member

Choose a reason for hiding this comment

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

Ignore the renaming comment

Copy link
Contributor

@geeknoid geeknoid left a comment

Choose a reason for hiding this comment

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

Let's use v1beta1 for consistency with the standard pattern.

Do the same for mixer/adapter/model too.
@guptasu guptasu changed the title Create policy/v1beta protos. Create policy/v1beta1 protos. Feb 16, 2018
Copy link
Contributor Author

@guptasu guptasu left a comment

Choose a reason for hiding this comment

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

@rshriram @geeknoid thanks for the comment. changed v1beta -> v1beta1

@geeknoid geeknoid merged commit 24209c4 into istio:master Feb 17, 2018
@ldemailly
Copy link
Member

I'm trying to see how that should reflect in
https://github.com/istio/vendor-istio/tree/master/istio.io/api
and strangely running dep ensure despite the bot having changed the sha in istio/istio toml/lock file doesn't seem to change anything there - any idea why ?
which file do you expect to change from this PR ?

ldemailly added a commit to istio/istio that referenced this pull request Feb 18, 2018
istio/old_vendor-istio_repo#2 is merged

Ran dep ensure
A bit surprised the api SHA change didn’t trigger any file change
istio/api#376 (comment)
@ldemailly
Copy link
Member

I think I know why: it's pruned / not used yet

We should change the bot a bit to also update vendor and the config to not prune istio.io/api
I'll file a task and get the later done

ldemailly added a commit to istio/istio that referenced this pull request Feb 18, 2018
* vendor_update_Feb17_2018

Ran dep ensure —update

Also bump up fortio version

Fixes #3585

* Update to head

istio/old_vendor-istio_repo#2 is merged

Ran dep ensure
A bit surprised the api SHA change didn’t trigger any file change
istio/api#376 (comment)
// $front_matter: redirect_from: /docs/reference/policy/policy-and-telemetry-rules.html

// Describes the rules used to configure Mixer's policy and telemetry features.
package istio.policy.v1beta;
Copy link
Contributor

Choose a reason for hiding this comment

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

why it's not v1beta1? no plan to make incompatible changes in beta?

@guptasu
Copy link
Contributor Author

guptasu commented Feb 20, 2018 via email

@xiaolanz
Copy link
Contributor

Yes. it looks great. If we plan to move CRDs any time soon, please coordinate with @cmluciano

incfly pushed a commit to incfly/api that referenced this pull request Jun 13, 2018
nacx pushed a commit to nacx/api that referenced this pull request Feb 23, 2022
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
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.

6 participants