Duplicate all protos from udpa tree to xds tree#17
Conversation
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
Who do you want to test for breakage?
| ) | ||
|
|
||
| # Old names for backward compatibility. | ||
| # TODO(roth): Remove these once all callers are migrated to the new names. |
There was a problem hiding this comment.
This will be removed in this PR?
There was a problem hiding this comment.
I was thinking that these build rules might be pulled in by projects that use this repo, so I didn't know if I could remove the old names without breaking anyone. But if we know that no one is using these rules, then I'm happy to remove them immediately.
| @@ -0,0 +1,38 @@ | |||
| syntax = "proto3"; | |||
There was a problem hiding this comment.
I assume you'll do another pass where you flag in bold letters that the old UDPA version is frozen, stale etc?
Signed-off-by: Mark D. Roth <roth@google.com>
markdroth
left a comment
markdroth
left a comment
There was a problem hiding this comment.
I was thinking about tests for projects that depend on this. I can obviously figure out whether this will break gRPC, but, for example, how do I know if this is going to be okay for Envoy?
| ) | ||
|
|
||
| # Old names for backward compatibility. | ||
| # TODO(roth): Remove these once all callers are migrated to the new names. |
There was a problem hiding this comment.
I was thinking that these build rules might be pulled in by projects that use this repo, so I didn't know if I could remove the old names without breaking anyone. But if we know that no one is using these rules, then I'm happy to remove them immediately.
| @@ -0,0 +1,38 @@ | |||
| syntax = "proto3"; | |||
|
@markdroth try build Envoy against this by updating repository_locations.bzl to point at the branch on your fork. If that works, I think things are good to go. We should ideally have CI support here one day. |
|
I ran this on istio/istio and there were no errors fwiw |
|
I haven't had a chance to test this with Envoy, but if Istio is happy with this, that's probably a good sanity check. And given that we're not actually removing any of the old protos, the odds of breakage are unlikely. So I'll go ahead and merge this. |
|
FWIW I only checked our go codebase, not our Envoy extension build, so it
plausibly could impact Envoy. But I think its probably safe
…On Mon, Oct 11, 2021 at 10:35 AM Mark D. Roth ***@***.***> wrote:
Merged #17 <#17> into main.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#17 (comment)>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYGXNIV6XDETQG7ACMJ4LUGMN7FANCNFSM5FRXOR6Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
3 participants
This is step 1 of the plan from #2.
Question: How do I test something like this to make sure that it's not going to break any callers?