-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pgv: bump dependency to 30da78c. #4551
Conversation
This should unblock Windows, fix the long CLI issue and also fuzz weak link problems. Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
@@ -1,13 +1,20 @@ | |||
BAZEL_SKYLIB_SHA = "b5f6abe419da897b7901f90cbab08af958b97a8f3575b0d3dd062ac7ce78541f" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to be consistent with other dependencies what about:
BAZEL_SKYLIB_RELEASE = "0.5.0"
BAZEL_SKYLIB_SHA = "b5f6abe419da897b7901f90cbab08af958b97a8f3575b0d3dd062ac7ce78541f"
api/bazel/repositories.bzl
Outdated
name = "bazel_skylib", | ||
url = "https://github.com/bazelbuild/bazel-skylib/archive/0.5.0.tar.gz", | ||
sha256 = BAZEL_SKYLIB_SHA, | ||
strip_prefix = "bazel-skylib-0.5.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And
sha256 = BAZEL_SKYLIB_SHA,
strip_prefix = "bazel-skylib-" + BAZEL_SKYLIB_RELEASE,
url = "https://github.com/bazelbuild/bazel-skylib/archive/" + BAZEL_SKYLIB_RELEASE + ".tar.gz",
Signed-off-by: Harvey Tuch <htuch@google.com>
FYI, with this PR I've been able to upgrade to |
It looks like we'll need one more set of changes to PGV to get it to work on Windows; we've submitted bufbuild/protoc-gen-validate#104 The issue is the envoy/api/envoy/api/v2/core/base.proto Line 128 in 0c4c00e
Specifically, |
@sesmith177 ack. Are we good to merge this PR as an intermediate step? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
yep, we'll PR the update to PGV. It also looks like this comment is no longer true now: envoy/api/bazel/api_build_system.bzl Lines 127 to 130 in 0c4c00e
|
@sesmith177 Yes the comment should be removed. In @akonradi's earlier diff's on this PR he had removed those 4 lines. |
bufbuild/protoc-gen-validate#104 has been merged |
Signed-off-by: Harvey Tuch <htuch@google.com>
This should unblock Windows, fix the long CLI issue and also fuzz weak link problems.
Signed-off-by: Harvey Tuch htuch@google.com