Add CI step to build using protoc with fatal warnings#237
Conversation
ce88bd0 to
6a0add9
Compare
6a0add9 to
fcfa6d2
Compare
The steps in the CI file from the step `nox` onwards miss some cases, like unused imports. An additional step with `protoc` takes care of this. This step generates the protoc files directly using `protoc`, and treats warnings as fatal. Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
fcfa6d2 to
48da29a
Compare
protocprotoc with fatal warnings
matthias-wende-frequenz
left a comment
There was a problem hiding this comment.
Looks good but I wonder if that checks should be moved to nox and repo-config. @llucax Opinions?
Right, this should be added to repo-config at some point. Also, the action for setting up protoc has to be extracted into a separate action. |
llucax
left a comment
There was a problem hiding this comment.
Just a minor comment, but I'm also fine to keep it as it is.
repo-config is needing some love, we are not being able to catch-up with the pace of new repo types (like api clients), and I want to split it in smaller repos (one for generating protobuf, one with the tools for mkdocs, one with just the cookicutter templates, etc.). We want to remove the python code generation too soon ™️ from "api" repos, so there is a lot of work to do on that side too. So I think we can wait for all that restructuring before pushing this to repo-config. Eventually I think we'll need to do a sprint to re-repo-configy all repositories. |
Yeah, that too, I started the process of splitting the big |
The steps in the CI file from the step
noxonwards miss a few cases, like reporting unused imports. An additional step withprotoctakes care of this. This step generates the protoc files directly usingprotoc, and treats warnings as fatal.Closes #228