-
Notifications
You must be signed in to change notification settings - Fork 581
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
Update bazel and deps #1111
Update bazel and deps #1111
Conversation
261e913
to
984c7b5
Compare
|
||
# gazelle:repository_macro dependencies.bzl%go_third_party | ||
go_third_party() | ||
pinned_maven_install() |
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.
this is still a bit of a mess since you cannot do some of these loads in the same bzl file where you pull in the dep, I think we'd have to create another file if we wanted to eliminate all the custom things done here.
name = "pgv_pip_deps", | ||
requirements = "@com_envoyproxy_protoc_gen_validate//python:requirements.in", | ||
requirements_lock = "@com_envoyproxy_protoc_gen_validate//python:requirements.txt", |
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.
the old rule was removed the new one requires a lockfile
go_third_party() | ||
|
||
maven_install( | ||
artifacts = PROTOBUF_MAVEN_ARTIFACTS, |
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.
this is required for protobuf java code now
@@ -238,4 +238,5 @@ java_proto_gen_validate = rule( | |||
"srcjar": "lib%{name}-src.jar", | |||
}, | |||
implementation = _java_proto_gen_validate_impl, | |||
toolchains = ["@bazel_tools//tools/jdk:toolchain_type"], |
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.
@@ -35,9 +35,5 @@ cc_test( | |||
|
|||
py_proto_library( | |||
name = "bar_py_proto", | |||
srcs = ["bar.proto"], | |||
deps = [ |
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.
py_proto_library from rules_python now works how all the others do
@@ -9,7 +9,7 @@ public final class RequiredValidation { | |||
private RequiredValidation() { | |||
} | |||
|
|||
public static void required(String field, GeneratedMessageV3 value) throws ValidationException { |
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.
"requirements.in", | ||
"setup.cfg", | ||
]) | ||
|
||
compile_pip_requirements( |
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.
this also creates a test to make sure the lockfile is updated, so the new lockfile is slightly easier to manage with that
This modernizes the bazel setup to support 7.x, still using the WORKSPACE
bb398ce
to
e60389d
Compare
@rodaine could you take a look at this one? I just rebased but looks like you need to approve the CI run |
thanks! pushed a fix for that failure, hopefully that's the only one! |
green 🎉 |
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.
Thank you so much, @keith!
This modernizes the bazel setup to support 7.x, still using the WORKSPACE