refactor(evpn-gw): updates to support buf lint and generation #411
Conversation
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
network/evpn-gw/Makefile
Outdated
| rm -rf ./google | ||
| rm -rf ./v1alpha1/{autogen.md,gen,google} | ||
| mkdir -p ./v1alpha1/gen/{go,cpp,python,java} | ||
| rm -rf ./v1alpha1/{autogen.md} |
There was a problem hiding this comment.
{} are not needed since we have only one option autogen.md
Also, since it is a single file, we can just use rm -f
network/evpn-gw/Makefile
Outdated
| @@ -3,22 +3,22 @@ | |||
| # Copyright (c) 2022 Dell Inc, or its subsidiaries. | |||
| # Copyright (C) 2023 Nordix Foundation. | |||
|
|
|||
| all: | |||
| all: buflint doc | |||
There was a problem hiding this comment.
do we intentionally not run bufgen target? Without bufgen, the CI job does not actually check that we provided all generated artifacts
opi-api/.github/workflows/network.yml
Lines 42 to 48 in b990048
I missed that point on the review of opinetcommon as well...
There was a problem hiding this comment.
Yes, I will add that into the steps currently. Eventually the generated files will be placed into a separate repository and autogenerated; but, for now adding the bufgen in the make should take care of the operation.
There was a problem hiding this comment.
@sandersms I guess you are doing the transition to the buf lint but you still need to generate the go/python files by yourself. Right ? When we move the apis to their own repo then the generation of the go/python files will happen automatically. So this is a prepatory step. Right ?
| @@ -337,9 +337,9 @@ enum BPOperStatus { | |||
| // BridgePortType reflects the different types of a Bridge Port | |||
| enum BridgePortType { | |||
| // "unknown" bridge port type | |||
| UNKNOWN = 0; | |||
| BRIDGE_PORT_TYPE_UNSPECIFIED = 0; | |||
There was a problem hiding this comment.
What is the reason that we have changed the ENUM here ? Does this follows the googles AIPs for ENUMs ?
There was a problem hiding this comment.
Yes - this follows the AIPs for ENUMs as part of the checking. It also requires the addition of the "BRIDGE_PORT_TYPE_" for all of the ENUMs to be unique as a check.
@mardim91 Yes, this is the preparatory step to get to the generation of the apis and I will be adding the buf gen to the full make to have the go/python files generated as part of this PR as noted in one of the earlier comments. We will be moving the apis that are generated to their own repo once we get all of the capabilities in place. |
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
| @@ -3,3 +3,11 @@ name: buf.build/opiproject/evpn-gw | |||
| deps: | |||
| - buf.build/googleapis/googleapis | |||
| - buf.build/grpc-ecosystem/grpc-gateway | |||
| - buf.build/opiproject/opinetcommon | |||
| lint: | |||
There was a problem hiding this comment.
let's also include COMMENTS linter rules here
There was a problem hiding this comment.
Added to latest addition.
artek-koltun
dismissed
their stale review
let's also include COMMENTS linter rules here
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
sandersms
force-pushed
the
evpn-automate
branch
from
564d6d3
to
e0ec268
Compare
Transition evpn-gw to the buf lint and buf generate operation to allow for automation of the protobuf file generation.
Support for cpp and java are being removed at this time. Generation supports go and python. Support for other languages can be added as needed by the buf.gen.yaml file.
Be aware that the .proto files are moved up from the v1alpha1 directory to support the buf operation and to keep the version of the generated files in a separate (v1, v1alpha1, etc.) directory structure. This will allow for the future ability to autogenerate the files and place them in a separate repo location.